New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use ConcurrentWeakHashMap for RubyClass.subclasses #5137

Merged
merged 2 commits into from Apr 12, 2018

Conversation

Projects
None yet
1 participant
@headius
Member

headius commented Apr 12, 2018

The naively-synchronized WeakHashSet does not guarantee safe iteration in the presence of modification, which triggers a ConcurrentModificationException as seen in #5115.

By using a ConcurrentWeakHashMap (designed by Doug Lea et al) we can iterate and modify concurrently without errors.

However this implementation is based the design of ConcurrentHashMap, which means the in-memory structure will likely be considerably larger than a simple WeakHashMap. I believe it is also based off the pre-Java 8 ConcurrentHashMap, which means any load factor above one thread greatly increases in-memory size, and a load factor of one limits the concurrent use of the collection.

Use ConcurrentWeakHashMap for RubyClass.subclasses.
The naively-synchronized WeakHashSet does not guarantee safe
iteration in the presence of modification, which triggers a
ConcurrentModificationException as seen in #5115.

By using a ConcurrentWeakHashMap (designed by Doug Lea et al, see
http://www.java2s.com/Code/Java/Collections-Data-Structure/Ahashtablewithemweakkeysemfullconcurrencyofretrievalsandadjustableexpectedconcurrencyforupdates.htm)
we can iterate and modify concurrently without errors.

However this implementation is based the design of
ConcurrentHashMap, which means the in-memory structure will likely
be considerably larger than a simple WeakHashMap. I believe it is
also based off the pre-Java 8 ConcurrentHashMap, which means any
load factor above one thread greatly increases in-memory size, and
a load factor of one limits the concurrent use of the collection.

@headius headius added this to the JRuby 9.1.17.0 milestone Apr 12, 2018

@headius headius requested review from kares and enebo Apr 12, 2018

@headius

This comment has been minimized.

Member

headius commented Apr 12, 2018

It's hardy scientific, but using VisualVM to get a heap dump of the following script:

a = Object
10000.times { a = Class.new(a) }
puts 'ok'
sleep

Shows a bit over a 2MB size difference, or about 200 bytes per class. This may increase with more subclasses in each collection, but it should be a much smaller increment than the base hit of the collection.

Note that I'm forcing a load factor of 1, so we are getting no concurrency out of this collection...just thread-safe traversal+modification.

Before:
image

After:
image

@headius

This comment has been minimized.

Member

headius commented Apr 12, 2018

This is a pretty non-invasive patch, and the cost of 200 bytes per class doesn't seem too bad, so I vote that we go with it.

@headius headius merged commit 306af19 into jruby-9.1 Apr 12, 2018

@headius headius deleted the concurrent_subclasses branch Apr 12, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment