Issue2230

Basically the PyType class is caching lookups but under 60k/s evaluations on 24 core machines it causes a great deal of contention so much so it locks the application. Using the high-scale-lib the lock contention is removed and tests show 80k/s evals.

Is there any way that you could reduce this to a simple code fragment that illustrates the problem?

Makes sense. Let's see if we can apply post beta 4, in time for the RC.

I looked into this requested change. The problem is that NonBlockingIdentityHashMap does not support weak keys/weak values, which is necessary to prevent memory leaks.

Perhaps the best solution here is to allow the user to provide their own instance for classToBuilder, while maintaining the current behavior as default.

Such a change would be done at the Java integration level, vs the current convenience wrapper of the Jython bash script.

Putting what I suggested (msg9314) on the backlog for 2.7.1 - we should offer more tuning opportunities for Jython, and supporting it is an easy win.

William, one thing: did you try changing the concurrency level in MapMaker? We are using the default, which is 4, which may explain the lock contention you are seeing. If you haven't, could you try running your performance tests with a higher level?

Per the docs, "a significantly lower value can lead to thread contention"
http://docs.guava-libraries.googlecode.com/git/javadoc/com/google/common/collect/MapMaker.html#concurrencyLevel(int)

So you might want to try something like

    public static synchronized PyType fromClass(Class<?> c, boolean hardRef) {
        if (class_to_type == null) {
            class_to_type = new MapMaker().concurrencyLevel(16).weakKeys().weakValues().makeMap();
            addFromClass(PyType.class, null);
        }

We might want to make this the default regardless, given that this is a singleton.

I'll give it a shot, there was several issues.

1. Stacktraces in jython are very expensive.. so logging was deceptively contributing to slow execution.

2. The code we used built a javax.script.CompiledScript for code isolation.. but it ended up being slow so we switch to creating functions and using invoke..

I'll need to go back and see if that solves other issues.. but the main issue is is this code in our environment was being executed at a very high rate... I think I've reduced that rate and therefore its not as big of a deal.

..
As far as previous comments about the solution path.. I choose NonBlockingIdentityHashMap because .class objects should already be in perm gen and this would simply store references so it, should be a pretty small memory footprint basically just pointers.. and while weakkeys allows one to unload low volume references.. IHMO the footprint was so small that the application probably has bigger issues.. a bit extra memory for holding the references.. there was no discernible difference in our application.

William, thanks for taking a look at this. Re your points:

1. Stack traces in Java are expensive when compared to simply using exceptions. But we are certainly making this be even more expensive in Py#getStackTrace, which is simply ancient code in Jython that parses the stack trace string representation, instead of using http://docs.oracle.com/javase/7/docs/api/java/lang/Throwable.html#getStackTrace(), available since Java 1.4. (hg blame shows that this code still has parts unchanged from when Jim Huginin and Barry Warsaw worked on it 16-17 years ago!)

2. If at all possible, you want to use code objects that are already compiled and invoke them, vs recompiling every single time. Basically every time you eval/compile something on Jython, no matter how small, it creates a class and loads it with a classloader. So I think this is the right strategy, even if it means we don't have such a good performance test :)

3. Re weak references - this is certainly true of your scenario, however, it causes memory leaks for other usage. So this seems to be the best compromise. Providing a pluggable map for class_to_type would be even better of course.

Tracebacks are even more expensive than I thought. If you look at PyTraceback.java, you can see that PyTraceback#getLine does not even attempt to cache the line, or optimize access to the file by getting multiple lines.

So this means we should at least port Lib/linecache.py to Java to speed things up (https://docs.python.org/2/library/linecache.html), as well as avoid a dependency in Python code for something as critical as the traceback.

On point 2

I was caching the CompiledScript and using Bindings to pass in variables. I didn't realize it still required re-compiling or additional processing.

The use case was for user provided expressions..

Example:
x AND y

My assessment was dynamically wrapping the expression in a function could potentially be error prone as the language syntax depends on line spacing.

However the performance difference was so great that I ended up doing it anyway.. and sure enough there were some sample expressions I had to modify but it wasn't as bad as I anticipated.

So I'm dynamically turning the expression above into..

def fx(x,y):
 return x AND y

Addendum: 

I found out this was also true for other scripting languages, Groovy in particular.

Ack. CPython has been compiled into Python bytecode since the earliest release, nearly 25 years ago; and this has also been true of Jython (to Java bytecode), IronPython (to CLR), and PyPy (to Python bytecode, then JITed). Of popular JVM languages, I believe only JRuby attempts to do a classical AST interpreter (but does do a tiered compilation strategy for hot code).

Revisit tuning, possibly providing some user configuration, per msg9623

Based on the diff from William, the problem is being addressed by Jeff Allen's fix here: https://bitbucket.org/tournesol/jython-fgtype/

We expect this fix to be included in 2.7.2

PyType has changed a lot in how it addresses the issues discussed here. It would be useful to know whether the current tip, or subsequent 2.7.2b runs in the environment described. But that was 3 years ago ...

Leaving tagged at 2.7.2, hopefully.

Assuming the OP's original analysis is correct, that contention for the cache in PyType is at the root of this, we expect the switch to ClassValue (https://hg.python.org/jython/rev/55756721fa02) to have resolved this issue at scale.

2.7.2 beta may provide experience with something like the hardware described here, to confirm or disprove this optimism.

Given time elapsed and feedback on #2834, I think this is properly fixed.