Optimize the use of AtomicReferenceFieldUpdaters #8
Conversation
|
Thanks for your PR. Could you report about performance when this changes is applied? How much better is it now? |
|
No problem. It seem's Moiz's Serializable patch broke the tests (I am getting a StackOverflowError). I will investigate/patch and report back as soon as I can. From: romix notifications@github.com Thanks for your PR. Could you report about performance when this changes is applied? How much better is it now? Reply to this email directly or view it on GitHubhttps://github.com//pull/8#issuecomment-55012934. Mlynské Nivy 56 / 821 05 Bratislava / Slovakia |
Internal constructor needs to pass down the arguments, so read-only snapshots are created properly. This allows serialization to work as expected. Also add serialVersionUID to fix some warnings. Signed-off-by: Robert Varga <robert.varga@pantheon.sk>
Benchmarks indicate that instatiating the updaters takes ~92% of the CPU time required to initialize a TrieMap instance. TrieMap.inodeupdater is an unused per-object updater, which has its equivalent in INodeBase.updater. rtupd is an unused istance, so we can freely remove it, lowering the memory cost of a TrieMap just a tiny bit. Finally, we centralize the allocation of rootupdater -- instead of allocating it over and over again, we have a single instance which we share. Signed-off-by: Robert Varga <nite@hq.sk>
Couple of improvements to the serialization strategy: - emit the fact that the map is read-only, so we preserve that trait - write out the size and entries ourselves, not via an intermediate HashMap() The read path now takes these changes into account and resets the rootupdater after deserialization is done back to null. Signed-off-by: Robert Varga <robert.varga@pantheon.sk>
Removes duplicate fields and makes the equivalent methods final. Also marks things that we take care of as transient, thus allowing us to reuse the JVM magic in serialization. Signed-off-by: Robert Varga <robert.varga@pantheon.sk>
|
Ugh, sorry for the slew of patches ... I can split them up into multiple merges (still learning here). |
This reorganizes the initialization a bit and makes sure, that root node is initialized even by the default constructor. Signed-off-by: Robert Varga <robert.varga@pantheon.sk>
Instead of using the updater reference as the indicator of read-only views, add an explicit boolean. Thus all instances reuse a single updater reference, but still guard access to it via an explicit check. This changes the behavior a bit: instead of throwing NPE on read-only views, we throw IllegalStateException. The reason for that is that we should have caught the attempt in the MAP API functions. Signed-off-by: Robert Varga <robert.varga@pantheon.sk>
Adds explicit checks for read-only snapshot before attempting to perform a modification operation. As per ConcurrentMap/Iterator interface specification, throw an UnsupportedOperationException if such an attempt is made. Signed-off-by: Robert Varga <robert.varga@pantheon.sk>
|
Yes. It would be nice if you split it into separate patches. The ones related to serialization and those related to atomics.
Why is it tested with different number of initializations? Couldn't it be done for the same number just for the sake of comparing apples to apples? |
|
Closing this one, will reopen a few point PRs. |
Benchmarks indicate that instatiating the updaters takes ~92% of the CPU
time required to initialize a TrieMap instance.
TrieMap.inodeupdater is an unused per-object updater, which has its
equivalent in INodeBase.updater.
rtupd is an unused istance, so we can freely remove it, lowering the
memory cost of a TrieMap just a tiny bit.
Finally, we centralize the allocation of rootupdater -- instead of
allocating it over and over again, we have a single instance which we
share.
Signed-off-by: Robert Varga nite@hq.sk