Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add out-of-process JIT support #1561

Merged
merged 197 commits into from
Sep 17, 2016
Merged

Add out-of-process JIT support #1561

merged 197 commits into from
Sep 17, 2016

Conversation

MikeHolman
Copy link
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
Copy link
Contributor

dilijev commented Sep 14, 2016

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
Copy link
Contributor

dilijev commented Sep 14, 2016

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
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
Copy link
Contributor

dilijev commented Sep 14, 2016

@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
Copy link
Contributor

dilijev commented Sep 14, 2016

@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
Copy link
Contributor

dilijev commented Sep 15, 2016

@dotnet-bot test nojit tests please

@dilijev
Copy link
Contributor

dilijev commented Sep 16, 2016

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
Copy link
Contributor Author

@dotnet-bot test nojit tests

@MikeHolman
Copy link
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
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
Copy link

jdalton commented Dec 22, 2016

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