oracle / graalpython

GraalPy – A high-performance embeddable Python 3 runtime for Java
https://www.graalvm.org/python/
Other
1.26k stars 113 forks source link

Python context isolation broken? #20

Open revintec opened 6 years ago

revintec commented 6 years ago
import org.graalvm.polyglot.Context;
import org.graalvm.polyglot.Engine;
import org.graalvm.polyglot.Source;

/** Created by revin on Oct.1,2018. */
public class BugReport{
    public void trace(Object x){System.out.println(x);}
    public static void main(String...args)throws Exception{
        Engine engine=Engine.create();
        Context c0=Context.newBuilder().allowIO(true).engine(engine).build();
        Context c1=Context.newBuilder().allowIO(true).engine(engine).build();

        Source source=Source.newBuilder("python",
                "import polyglot\n"+
                "try:bug.trace(\"SHOULD NOT SEE THIS\")\nexcept:pass\n"+
                "bug=polyglot.import_value(\"bug\")\n"
                ,"bugReport.py").build();

        BugReport bug=new BugReport();
        long nano0=System.nanoTime();c0.getPolyglotBindings().putMember("bug",bug);c0.eval(source);
        long nano1=System.nanoTime();c1.getPolyglotBindings().putMember("bug",bug);c1.eval(source);
        long nano2=System.nanoTime();System.out.println((nano1-nano0)/1000_000+" ms without init, "+(nano2-nano1)/1000_000+" ms after init");
    }
}

image

expected output: none actual output: "SHOULD NOT SEE THIS"

as you can see from the simplified code above: it creates two separate contexts, run the same code in each one of them but it seems somehow the global variable bug leaked from the first context into the second one

am I misunderstood something here? how can I achieve performant context isolation? context initialization(even if they use the same engine instance underneath) seems very heavy

lukasstadler commented 6 years ago

Hi! The false sharing between the context is definitely a bug, we will take a look at that. It was likely introduced with some recent changes for sharing ASTs between contexts.

The context creation should get significantly faster after a couple of contexts have been created and the JVM had some time to warm up. You could also try and create a native image for your Java application, which removes the JVMs warmup altogether.

revintec commented 6 years ago

@lukasstadler Hi, thx for the quick reply

my use case is like the CGI model: the Java program acts like a HTTP server, when a request is received from client it creates a new context, some python code runs inside the newly created context and generates a response

as you can see, creating a new context each time is kinda essential so that different requests won't get mess up, but as I've discovered that creating many short-lived context is very heavy, maybe this is not how contexts're supposed to be used? how can I do better?


I've created a new file for a quick poor-man's profiling: it runs very slow, GC, mem and CPU are all very high after 4 minutes, it still costs about 400ms each run

import org.graalvm.polyglot.Context;
import org.graalvm.polyglot.Engine;
import org.graalvm.polyglot.Source;

/** Created by revin on Oct.1,2018. */
public class BugReport{
    public static void main(String...args)throws Exception{
        Engine engine=Engine.create();
        Source source=Source.newBuilder("python","pass","bugReport.py").build();
        for(int i=0;i<0x10000;++i){
            long nano0=System.nanoTime();
            Context.newBuilder().allowIO(true).engine(engine).build().eval(source);
            System.out.printf("%7d ms%n",(System.nanoTime()-nano0)/1000_000);
        }
    }
}

the following images are gathered from Java Mission Control

image image

lukasstadler commented 6 years ago

The false sharing is fixed with https://github.com/graalvm/graalpython/commit/cb7b3fde0c3e2c633285b412b766ad03ae415079.

Indeed, the context initialization should be faster, at least after a couple of warmup runs. There's some issues around how classes are handled when multiple contexts are used, I'm taking a look at that at the moment. One effect of that is that the compiled code size gets very large, which causes the high load on the compiler threads you're seeing.

revintec commented 6 years ago

@lukasstadler Any update on this?

lukasstadler commented 6 years ago

Uh, sorry that I didn't provide an update on this. With the latest release, context initialization time goes below 100ms after a while on my (rather slow) machine, so much better but still not perfect. There may be theoretical limits on how fast this can be, since context initialization does lots of things. Starting up a context in CPython (time python -c 'print("foo")') takes ~65ms. I guess the quicker the better, but what amount of time would make sense in your case? Are the scripts very short-lived?

revintec commented 6 years ago

@lukasstadler thanks for your reply, I'll redo my tests asap with the newest version

my use case is like the CGI model: the Java program acts like a HTTP server, when a request is received from client it creates a new context, some python code runs inside the newly created context and generates a response

the scripts are indeed very short-lived, ideally all of them are less than 100ms I'm currently using thread-local cached contexts, but that is a compromise I think, because some scripts may leak to or even change global scope which may affect other following scripts or cause memory leaks. using a fresh context each time and close each of them after each script has been run should be cleaner I think

I'm not very familiar with python (especially its initialization process), but after reading (some of) the source code, it seems that a lot of initialization is done when the python context is being created. maybe we can remove some of the code, or is there a way like to create a templateContext and shallow copy them to newly created contexts? without affecting context isolation of course