{"action":"created","issue":{"url":"https://api.github.com/repos/ruby-concurrency/concurrent-ruby/issues/281","labels_url":"https://api.github.com/repos/ruby-concurrency/concurrent-ruby/issues/281/labels{/name}","comments_url":"https://api.github.com/repos/ruby-concurrency/concurrent-ruby/issues/281/comments","events_url":"https://api.github.com/repos/ruby-concurrency/concurrent-ruby/issues/281/events","html_url":"https://github.com/ruby-concurrency/concurrent-ruby/pull/281","id":70948857,"number":281,"title":"AtomicMarkableReference: Add pure Ruby implementation","user":{"login":"ianks","id":3303032,"avatar_url":"https://avatars.githubusercontent.com/u/3303032?v=3","gravatar_id":"","url":"https://api.github.com/users/ianks","html_url":"https://github.com/ianks","followers_url":"https://api.github.com/users/ianks/followers","following_url":"https://api.github.com/users/ianks/following{/other_user}","gists_url":"https://api.github.com/users/ianks/gists{/gist_id}","starred_url":"https://api.github.com/users/ianks/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ianks/subscriptions","organizations_url":"https://api.github.com/users/ianks/orgs","repos_url":"https://api.github.com/users/ianks/repos","events_url":"https://api.github.com/users/ianks/events{/privacy}","received_events_url":"https://api.github.com/users/ianks/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":29,"created_at":"2015-04-25T18:21:51Z","updated_at":"2015-06-05T19:13:46Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/ruby-concurrency/concurrent-ruby/pulls/281","html_url":"https://github.com/ruby-concurrency/concurrent-ruby/pull/281","diff_url":"https://github.com/ruby-concurrency/concurrent-ruby/pull/281.diff","patch_url":"https://github.com/ruby-concurrency/concurrent-ruby/pull/281.patch"},"body":"This is the beginning of adding the primitive concurrent type AtomicMarkableReference. It is much like the AtomicReference, but contains a boolean `marked` value. It will be neccesary for implementing the recursive split-order (lock-free) hash table I plan to implement.\r\n\r\nThe code is pretty straight forward, and steals mostly from AtomicReference; there are some refactorings possible there but I did not feel comfortable diffing too much code to make that happen on my first PR.\r\n\r\nSome notes/question about the design:\r\n\r\n  - I used Java's API as a guideline for implementing the API for this\r\n  - Like in Java, `get` returns an array of `[@value, @marked]`, I am not a huge fan of returning an array like that for a public facing API, but since this is low level enough it might be justified. Plus it is rather easy to destructure -> `markable_ref.get = val, mark`. Do you think returning an array here is idiomatic-enough Ruby? Obviously, returning the acutal object would be the *most* OO, but there are obvious race conditions there so maybe we are an exception to the rule here. An alternative would be to create some sort of `State` object which represents the given state of the object at the time of return. Thoughts?\r\n  - I noticed none of the tests in `atomic_markable_reference_spec` attempted to create possible race conditions, etc; but there are specs in other places which rely on AtomicReference to work. Is this the way you guys have been testing the implementation in a threaded environment? Testing multithreaded implmentations is tough, as tests tend to fall short of covering the cases. Do you guys think I should add some tests to document in a threaded environment? Or would some sort of informal proof be better? Sorry about ramblng here; but if you guys could give me some better insight about testing in this lib in general I would be super appreciative!\r\n\r\nPlease go crazy on the feedback for improvments here! I would love to optimize this code in any way possible :smiley:\r\n"},"comment":{"url":"https://api.github.com/repos/ruby-concurrency/concurrent-ruby/issues/comments/109407223","html_url":"https://github.com/ruby-concurrency/concurrent-ruby/pull/281#issuecomment-109407223","issue_url":"https://api.github.com/repos/ruby-concurrency/concurrent-ruby/issues/281","id":109407223,"user":{"login":"ianks","id":3303032,"avatar_url":"https://avatars.githubusercontent.com/u/3303032?v=3","gravatar_id":"","url":"https://api.github.com/users/ianks","html_url":"https://github.com/ianks","followers_url":"https://api.github.com/users/ianks/followers","following_url":"https://api.github.com/users/ianks/following{/other_user}","gists_url":"https://api.github.com/users/ianks/gists{/gist_id}","starred_url":"https://api.github.com/users/ianks/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ianks/subscriptions","organizations_url":"https://api.github.com/users/ianks/orgs","repos_url":"https://api.github.com/users/ianks/repos","events_url":"https://api.github.com/users/ianks/events{/privacy}","received_events_url":"https://api.github.com/users/ianks/received_events","type":"User","site_admin":false},"created_at":"2015-06-05T19:13:46Z","updated_at":"2015-06-05T19:13:46Z","body":"@pitr-ch I updated the `try_update` API as per your suggestion. I tend to like not raising an exception by default. The only outstanding issue is that it differs from the AtomicReference API. We should considering deprecating the old AtomicReference `try_update` in favor of the: `try_update -> false` and `try_update! -> exception` before 1.0. @jdantonio, thoughts on this?"}}