Performance of development snapshot

[sdismo]

Hi,

I have done some performance testing on development snapshot svn rev 151. There is a performance issue there that is not present in the 2.1 release. We are running on java 1.7.

Our application experiences rather severe lock contension in the Java ClassLoader. This seems to come from changes in the ObjectBinder class where method findClassInMap has been modified to take ContextClassLoaders into consideration. If there is a ContextClassLoader present then loadClass() on this loader is invoked rather than Class.forName().

The problem with this is that ClassLoader.loadClass() acquires a lock before calling native code whereas Class.forName() does not. In our application this lock quickly becomes a bottleneck.

I’m not sure that the change in ObjectBinder is correct, nor needed. I have always thought that you do not need to bother about ContextClassLoaders unless you are providing some extra class loading magic. The presence of a ContextClassLoader should go unnoticed by a user. If there is a ContextClassLoader then Class.forName() will take that into play under the hood. I have however not looked deep under the hood as all the nitty-gritty stuff takes place in native code.

Java class loading is a bit outside my regular scope of work. Everything said here may be completely wrong :-)

We are running on SE java 1.7 u6. There is no middleware between our application and FlexJSON.   

Our approach forward would be to remove any ContextClassLoader prior to invoking FlexJSON. The ContextClassLoader does not add any value at FlexJSON deserialization in our case. This would only be needed if next FlexJSON release behaves as svn rev 151.

I suspect that there are applications out there that may be hit a lot harder by this issue. 

Best Regards
Simon

[Charlie Hubbard]

This was a change made to address this bug report:

http://sourceforge.net/tracker/?func=detail&aid=3130753&group_id=194042&atid=947842

I've added a comment to the issue referencing this thread so the author of the bug report could provide us insight as to why he wants the getContextClassLoader() to be used as opposed to using Class.forName().

Charlie

[arch0njw]

I am no longer working the FlexJSON so I'm trying to recall the exact problem I was having.  I am pretty certain it had to do with custom classes in my JSON which were not being resolved with the Class.forName().

I'm sorry I cannot be more specific.  (And this is a good reminder to be more verbose in bug filing…)

[Charlie Hubbard]

arch0njw,

Do you remember what environment you were in?  Was it inside a web container?  If so which one…Tomcat? Weblogic?  If not what environment was it?  Did you create your own classloaders?  You may not remember the specifics about flexjson, but any details about the environment you saw this in might help us better understand how it could come to being.

Thanks,
Charlie

[Brandon Goodin]

I wanted to ping this thread and provide my thoughts.

I think we should roll the code back to using the old way of Class.forName(…). In the future if someone comes up with a real testable need for needing to access a different Classloader we can always setup a Classloader strategy.

In other projects we have provided the ability to pass in a Classloader. In this case we could allow the user to specify a different Classloader when setting up the JsonDeserializer instead of assuming the thread context class loader all the time. This would allow people to access a different ClassLoader if they need to. If a ClassLoader is provided then we could use the Class.forName(String name, boolean initialize, ClassLoader loader). Otherwise, we could use Class.forName(String className).

[Brandon Goodin]

Pinging this thread one more time to elicit a response. I'm planning to move ahead with my proposed change on this thread if there are no objections. I'd like to see us release a new version of FlexJson soon and this should be resolved before it goes out the door. Please respond with a message if you find the proposed change objectionable. Thanks all!

[allenru]

I have an example where the classloader is an issue.

We are rolling our own http session persistence implementation (for use within Jetty), and the implementation uses flexjson to serialize and deserialize the session data.

As you likely know, Jetty uses separate classloaders for each webapp (to sandbox them from each other) as well as it's own classloader for the application server libraries.  Well, the session persistence code (and flexjson) are loaded into the application server's classloader.  Meanwhile, the classes used in the session are loaded in the individual webapp classloaders.  Thus, when attempting to deserialize a session, flexjson can't see the webapp classes, as they are not in the classloader hierarchy of the application server.

I'm currently seeing this issue in version 2.1.  I am going to investigate the ContextClassLoader being discussed here and see if it fixes the class loading issue I am seeing.  If it does, then I will run a performance test.  After all, session persistence needs to be snappy.  I'll post back any findings.

Thanks!

[Charlie Hubbard]

I think passing a ClassLoader at init time is going to be an issue when you allow multiple threads to share the same JSONDeserializer.  Within the same Java application multiple threads could be using different ClassLoaders, and you really want to use the ClassLoader of the thread running the code right?

[Charlie Hubbard]

Well before we get all up in this with implementation.  I think we need to make sure we understand the differences we're deciding between:

http://www.javaworld.com/javaqa/2003-06/01-qa-0606-load.html

I have always thought that you do not need to bother about ContextClassLoaders unless you are providing some extra class loading magic. The presence of a ContextClassLoader should go unnoticed by a user.

This statement is completely false for the record.  We most certainly can have a problem not picking the right one.  We might have to do what Brandon is suggesting because this class loading stuff makes my head spin.  It was easy, now it's convoluted.  Anyway, I think we need to read the link, and think about threading and if we can pick a ClassLoader instance to provide it at construction time that will be happy for all threads.  My suspicion is sometimes Thread.getContextClassLoader() is the one you want to use as in allenru's case.

[allenru]

Using the context class loader, I was able to get the appropriate class loader for the target class.  In order to test this without rebuilding flexjson, I subclassed ObjectBinder and used it directly instead of going through JSONDeserializer.  Here is the rudimentary version of ObjectBinder using the contextClassLoader:

public class ClassLoaderAwareObjectBinder extends ObjectBinder {
        @Override
        protected Class findClassInMap(Map map, Class override) {
            if (override == null) {
                String classname = (String) map.remove("class");
                try {
                    if (classname != null) {
                        return Class.forName(classname, false, Thread.currentThread().getContextClassLoader());
                    } else {
                        return null;
                    }
                } catch (ClassNotFoundException e) {
                    return super.findClassInMap(map, override);
                }
            } else {
                return override;
            }
        }
}

I agree though, that this may not be the solution for everyone.  The JavaWorld article is a good reference on the subject.

-Russell