Sitelet https://github.com/chakra-core/ChakraCore/pull/1561
Skip to content

Add out-of-process JIT support - #1561

Merged
chakrabot merged 197 commits into
chakra-core:masterfrom
MikeHolman:oopjit
Sep 17, 2016
Merged

chakrabot merged 197 commits into
chakra-core:masterfrom
MikeHolman:oopjit

Conversation

@MikeHolman

Copy link
Copy Markdown
Contributor

This change adds support for hosts to optionally supply chakra with an external process (initialized with JsInitializeJITServer) to act as a JIT Server. A JIT server supports running any number of Chakra runtime clients.

The basic idea is that we have separated the JIT and runtime via an RPC interface, which is called in place of Func::CodeGen (when out-of-process JIT is enabled).

The CodeGen interface takes a workitem datastructure, containing all the information necessary to JIT a function (bytecode, profile data, obj typespec info, etc.), and addresses of ScriptContext and ThreadContext specific data which live on the server.

These structures are initialized via RPC whenever the first workitem is created for a context. This lifetime of these data structures is maintained by the runtime. When a ThreadContext or ScriptContext is freed, it requests the server to free their respective datastructures as well.

Code pages are allocated and written to with VirtualAllocEx/WriteProcessMemory, and the CustomHeap for code pages is maintained by the server (it lives off of the ThreadContext datastructure).

When the JIT wants to heap allocate some data for the code to reference, we allocate it on the JIT process and copy it to a buffer when returning back from the codegen call. The JIT code references the data as an offset from the base of the buffer, who's address is on the entrypoint. Any allocated structures with pointer fields have an associated fixup function that gets called after RPC returns.

Also, note that in ChakraCore this is currently only available using testhooks.

MikeHolman and others added 30 commits January 5, 2016 15:54
…DL, so we to ensure that Chakra.JITIDL builds first.

This is accomplished by adding a reference link.
Chakra.JITClient should not base excluding hte precompiled header on just x64 builds and we shouldn't be gating defines in CommonDefins.h on if we're building x86.
currently the const table reading is done by ReadProcessMemory(). may change to unbox and marshal through RPC
the following simple case is working with OOP JIT now:

// need to run with -off:ObjTypeSpec because it's not implemented for OOP JIT yet
function foo(a)
{
    var num = new Number(100.1);
    return a+1.2+num;
}
print(foo(1));
print(foo(2));
print(foo("x")); // bailout
BailoutInlined is working now. stack walker need some other change to work, specificall need to make IsNativeAddress() working for pages allocated from JIT process

working sample:
//c:\chakra\Build\VcBuild\bin\x64_debug\jshost.exe -off:ObjTypeSpec -mic:1 -off:simplejit c:\work\inline.js

var b = "asdf";
var c = 0;
var d = 0;
function bar1(){return d+1;}
function bar()
{
 c++;
 if(c==5)
 {
    bar1();
    throw new Error();
 }
}
function foo(a, b)
{
    bar();
    if(a / b > b)
    {
        return a+b;
    }
    else
    {
        return b+a+1;
    }
}
try{
print(foo(1, 2));
print(foo(2, 1));
print(foo(2, 1));
print(foo("a", 1));
d="aa";
print(foo("a", 1));
print(c);
}catch(e)
{
print(e.stack);
}
@dilijev

dilijev commented Sep 14, 2016

Copy link
Copy Markdown
Contributor

I doubt any of the improvements are real improvements. As for the regressions, is there anything we can tune in way we do OOP JIT that could recover those losses or are these going to be unrecoverable regressions in favor of the improved security?

@dilijev

dilijev commented Sep 14, 2016

Copy link
Copy Markdown
Contributor

Is there any perf impact from the previous version when these changes are in the code but OOP JIT is not turned on?

@MikeHolman

Copy link
Copy Markdown
Contributor Author

@dilijev The improvement in Richards at least has consistently showed up for months. Haven't looked much at others. My working theory is that worse JIT throughput has us rejit something slower, where the old code was better. Some of these losses are still recoverable, however this is within the realm of acceptable loss.

Thus far, we haven't seen any perf difference with oop jit disabled vs. master. There is some possibility of working set regression due to more data being copied at workitem creation, however in local traces we haven't seen this as an issue. But this is still pending verification from perf lab runs.

@dilijev

dilijev commented Sep 14, 2016

Copy link
Copy Markdown
Contributor

@MikeHolman Thanks for explaining the improvements. When you say consistently showing up for months, I assume you mean in this branch. It's interesting to see that OOP JIT could potentially improve performance in some scenarios.

I agree that the amount of the regressions doesn't seem especially concerning given the nature of this PR.

@dilijev

dilijev commented Sep 14, 2016

Copy link
Copy Markdown
Contributor

@MikeHolman I see that you needed to fix NoJIT build previously. Think it's a good idea to ask dotnet-bot for the full set those builds to show up on this PR to make sure they are all good?

https://github.com/Microsoft/ChakraCore/wiki/Jenkins-Build-Triggers#disablejit-builds

@dilijev

dilijev commented Sep 15, 2016

Copy link
Copy Markdown
Contributor

@dotnet-bot test nojit tests please

@dilijev

dilijev commented Sep 16, 2016

Copy link
Copy Markdown
Contributor

Review status: 0 of 296 files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.


bin/ch/ch.cpp, line 671 at r1 (raw file):

    if (i == argc)
        return false;

Please put braces around body of if-statement


Comments from Reviewable

@MikeHolman
MikeHolman force-pushed the oopjit branch 4 times, most recently from d8e0762 to 93a940f Compare September 16, 2016 03:01
@MikeHolman

Copy link
Copy Markdown
Contributor Author

@dotnet-bot test nojit tests

@MikeHolman

Copy link
Copy Markdown
Contributor Author

At suggestion of @EdMaurer I will be merging this in today to start getting test coverage. However, I will still very much appreciate CR feedback.

MikeHolman and others added 2 commits September 16, 2016 18:49
… directly use recycler to allocate the array that referencing to the numbers
@leirocks

Copy link
Copy Markdown
Contributor

@dotnet-bot test Ubuntu ubuntu_linux_release_static please

@chakrabot
chakrabot merged commit 9ab4101 into chakra-core:master Sep 17, 2016
@MikeHolman
MikeHolman deleted the oopjit branch September 29, 2016 21:28
@jdalton

jdalton commented Dec 22, 2016

Copy link
Copy Markdown

That's one big PR 😲 🥇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants