Issue 14 in objenesis: Unnecessary lock contention issues in ObjectInstantiator caching

[obje...@googlecode.com]

Status: New
Owner: ----

New issue 14 by singh.janmejay: Unnecessary lock contention issues in
ObjectInstantiator caching
http://code.google.com/p/objenesis/issues/detail?id=14

What steps will reproduce the problem?
1. Profile an application that calls ObjenesisBase#getInstantiatorOf(Class
clazz) several times across threads and observe high degree of lock
contention on that call.

What is the expected output? What do you see instead?
Expected behavior is, once cached, the act of getting the instantiator
should be lock-less.

What version of the product are you using? On what operating system?
1.2 (the patch is for master)

Please provide any additional information below.
The patch attached fixes this problem on trunk(it also cleanly applies on
the 1.2 tag). Its a git format-patch generated patch file, if working
directly with svn, 'patch -p1 -i <file-patch>' can be used to apply it.

Attachments:
0001-fixed-getInstantiatorOf-coarse-grained-lock-to-avoid.patch 3.0 KB

[obje...@googlecode.com]

Updates:
Status: Accepted
Labels: Milestone-1.3 Type-Defect

Comment #1 on issue 14 by henri.tremblay: Unnecessary lock contention

issues in ObjectInstantiator caching
http://code.google.com/p/objenesis/issues/detail?id=14

Thanks for this patch.

Your implementation is the clean way to do a double-checked locking. It is
guaranteed to work on Java 5 and 6. However, not on Java 1.3 and 1.4. Which
Objenesis still support. However, it
1) Might work anyway
2) Be useless to keep supporting these old java implementations
3) Might be possible to support both world nicely

Out of curiosity, why are you synchronizing on clazz.getName().intern()?
It's a nice way to prevent using a transient?

[obje...@googlecode.com]

Comment #2 on issue 14 by singh.janmejay: Unnecessary lock contention

issues in ObjectInstantiator caching
http://code.google.com/p/objenesis/issues/detail?id=14

I wanted something unique out of the class to take a lock(which doesn't
affect callers getting instantiator for a different class, hence
class.getName().intern(). I could have used class, but didn't want to worry
about multi-classloader environments(re-loading of class while locked etc).
Using name avoids all that trouble.

Does it make sense to branch out for java 1.4/1.3 and not apply this patch
there? I don't know, its a little bit of effort, but since the patch is
pretty localized, it may make sense. Just a suggestion.

[obje...@googlecode.com]

Comment #3 on issue 14 by dquintela: Unnecessary lock contention issues in
ObjectInstantiator caching
http://code.google.com/p/objenesis/issues/detail?id=14

class.getName().intern() is a recipe for disaster of memory leaks

[obje...@googlecode.com]

Comment #4 on issue 14 by dquintela: Unnecessary lock contention issues in
ObjectInstantiator caching
http://code.google.com/p/objenesis/issues/detail?id=14

well not exactly memory leaks, but memory exaustion

[obje...@googlecode.com]

Comment #5 on issue 14 by singh.janmejay: Unnecessary lock contention

issues in ObjectInstantiator caching
http://code.google.com/p/objenesis/issues/detail?id=14

It seems post java 1.2 it does get garbage-collected. Its a native call,
and i haven't checked how it works myself, but almost all pages i came
across indicate that interned strings are actually garbage collected(for
instance http://mindprod.com/jgloss/interned.html page talks about it).

[obje...@googlecode.com]

Updates:
Labels: -Milestone-1.3 Milestone-2.0

Comment #6 on issue 14 by henri.tr...@gmail.com: Unnecessary lock

contention issues in ObjectInstantiator caching
http://code.google.com/p/objenesis/issues/detail?id=14

(No comment was entered for this change.)

[obje...@googlecode.com]

Comment #7 on issue 14 by henri.tr...@gmail.com: Unnecessary lock

contention issues in ObjectInstantiator caching
http://code.google.com/p/objenesis/issues/detail?id=14

Here's a workaround I should have told way before.

You can do your own caching of the instances. For that, you should
desactivate the one from ObjenesisBase and just do your own.

You will still get a synchronized for any new class though. The next
version of Objenesis will not synchronize if the cache is disabled. Right
now, the best solution is to calling and instantiator used on your JVM
directly instead of using the strategy.


--
You received this message because this project is configured to send all
issue notifications to this address.
You may adjust your notification preferences at:
https://code.google.com/hosting/settings

[obje...@googlecode.com]

Updates:
Owner: henri.tr...@gmail.com

Comment #9 on issue 14 by henri.tr...@gmail.com: Unnecessary lock

contention issues in ObjectInstantiator caching
http://code.google.com/p/objenesis/issues/detail?id=14

(No comment was entered for this change.)

[obje...@googlecode.com]

Updates:
Labels: -Milestone-2.0 Milestone-1.4

Comment #8 on issue 14 by henri.tr...@gmail.com: Unnecessary lock

contention issues in ObjectInstantiator caching
http://code.google.com/p/objenesis/issues/detail?id=14

It is now possible to implement its own cache and so remove the
synchronization in Objenesis.

Version 2.0 will be only Java 5 compatible and will be able to really fix
this.

[obje...@googlecode.com]

Updates:
Status: Fixed

Comment #10 on issue 14 by henri.tr...@gmail.com: Unnecessary lock