Reduce locking in VariableTableManager#initObjectId #1400

Closed
nirvdrum opened this Issue Jan 13, 2014 · 1 comment

Projects

None yet

2 participants

@nirvdrum
Contributor
nirvdrum commented Jan 13, 2014 edited

While running a production Rails 3.2 app in JRuby 1.7.10 with 100 concurrent requests I saw a lot of blocked threads waiting on the lock in VariableTableManager#initObjectId. It appears that Rails makes excessive use of object_id and this seems to be hurting concurrency.

I'm seeing the source of contention in a couple places. One that pops up a fair bit is ActionView's template cache:

https://github.com/rails/rails/blob/v3.2.16/actionpack/lib/action_view/template/resolver.rb#L70-L90

Another seems to be in the Rails router when generating routes for the path/url helper methods. It seems to be in the journey gem, although the call to object_id isn't immediately clear:

https://github.com/rails/journey/blob/aa7e7438eb30516c3b72555e6f355c25d6a9c8ed/lib/journey/formatter.rb#L13-L47

@nirvdrum
Contributor

I tried removing the "synchronized" modifier from VariableTableManager#initObjectId and that didn't seem to blow things up. Of course, race conditions can be hard to surface. But it went away as a choke point.

@headius headius added a commit that closed this issue Jan 14, 2014
@headius headius Remove unnecessary synchronization from initObjectId. Fixes #1400.
The synchronization here is not actually necessary since the
variable accessor has already been allocated and the object's
monitor has already been acquired (both in getObjectId, the sole
caller of initObjectId). This synchronization caused global
bottlenecks against a given class, since it synchronized against
the VariableTableManager instance from that class. Since it's
likely that if user code acquires an object_id from one instance
of a class, it will acquire object_id from all instances of the
class...this quickly ran into multithreaded contention.

I opted to remove the synchronization, because I believe it is
safe, and modified initObjectId's visibility to be private so it
won't mistakenly be called by external code.
ce9b7f8
@headius headius closed this in ce9b7f8 Jan 14, 2014
@enebo enebo added this to the JRuby 1.7.11 milestone Feb 21, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment