-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
tools: js2c-cache #4777
Closed
Closed
tools: js2c-cache #4777
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,6 +13,7 @@ namespace node { | |
|
||
using v8::AccessType; | ||
using v8::Array; | ||
using v8::ArrayBuffer; | ||
using v8::Boolean; | ||
using v8::Context; | ||
using v8::Debug; | ||
|
@@ -40,6 +41,7 @@ using v8::ScriptCompiler; | |
using v8::ScriptOrigin; | ||
using v8::String; | ||
using v8::TryCatch; | ||
using v8::Uint8Array; | ||
using v8::UnboundScript; | ||
using v8::V8; | ||
using v8::Value; | ||
|
@@ -507,15 +509,35 @@ class ContextifyScript : public BaseObject { | |
Local<Integer> lineOffset = GetLineOffsetArg(args, 1); | ||
Local<Integer> columnOffset = GetColumnOffsetArg(args, 1); | ||
bool display_errors = GetDisplayErrorsArg(args, 1); | ||
MaybeLocal<Uint8Array> cached_data_buf = GetCachedData(env, args, 1); | ||
bool produce_cached_data = GetProduceCachedData(env, args, 1); | ||
if (try_catch.HasCaught()) { | ||
try_catch.ReThrow(); | ||
return; | ||
} | ||
|
||
ScriptCompiler::CachedData* cached_data = nullptr; | ||
if (!cached_data_buf.IsEmpty()) { | ||
ArrayBuffer::Contents contents = | ||
cached_data_buf.ToLocalChecked()->Buffer()->GetContents(); | ||
cached_data = new ScriptCompiler::CachedData( | ||
reinterpret_cast<uint8_t*>(contents.Data()), contents.ByteLength()); | ||
} | ||
|
||
ScriptOrigin origin(filename, lineOffset, columnOffset); | ||
ScriptCompiler::Source source(code, origin); | ||
Local<UnboundScript> v8_script = | ||
ScriptCompiler::CompileUnbound(env->isolate(), &source); | ||
ScriptCompiler::Source source(code, origin, cached_data); | ||
ScriptCompiler::CompileOptions compile_options = | ||
ScriptCompiler::kNoCompileOptions; | ||
|
||
if (source.GetCachedData() != nullptr) | ||
compile_options = ScriptCompiler::kConsumeCodeCache; | ||
else if (produce_cached_data) | ||
compile_options = ScriptCompiler::kProduceCodeCache; | ||
|
||
Local<UnboundScript> v8_script = ScriptCompiler::CompileUnbound( | ||
env->isolate(), | ||
&source, | ||
compile_options); | ||
|
||
if (v8_script.IsEmpty()) { | ||
if (display_errors) { | ||
|
@@ -525,6 +547,19 @@ class ContextifyScript : public BaseObject { | |
return; | ||
} | ||
contextify_script->script_.Reset(env->isolate(), v8_script); | ||
|
||
if (compile_options == ScriptCompiler::kConsumeCodeCache) { | ||
args.This()->Set( | ||
env->cached_data_rejected_string(), | ||
Boolean::New(env->isolate(), source.GetCachedData()->rejected)); | ||
} else if (compile_options == ScriptCompiler::kProduceCodeCache) { | ||
const ScriptCompiler::CachedData* cached_data = source.GetCachedData(); | ||
MaybeLocal<Object> buf = Buffer::Copy( | ||
env, | ||
reinterpret_cast<const char*>(cached_data->data), | ||
cached_data->length); | ||
args.This()->Set(env->cached_data_string(), buf.ToLocalChecked()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
EDIT: forget this for now. investigating something. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Want to share it with me? We could investigate together ;) |
||
} | ||
} | ||
|
||
|
||
|
@@ -677,6 +712,43 @@ class ContextifyScript : public BaseObject { | |
} | ||
|
||
|
||
static MaybeLocal<Uint8Array> GetCachedData( | ||
Environment* env, | ||
const FunctionCallbackInfo<Value>& args, | ||
const int i) { | ||
if (!args[i]->IsObject()) { | ||
return MaybeLocal<Uint8Array>(); | ||
} | ||
Local<Value> value = args[i].As<Object>()->Get(env->cached_data_string()); | ||
if (value->IsUndefined()) { | ||
return MaybeLocal<Uint8Array>(); | ||
} | ||
|
||
if (!value->IsUint8Array()) { | ||
Environment::ThrowTypeError( | ||
args.GetIsolate(), | ||
"options.cachedData must be a Buffer instance"); | ||
return MaybeLocal<Uint8Array>(); | ||
} | ||
|
||
return value.As<Uint8Array>(); | ||
} | ||
|
||
|
||
static bool GetProduceCachedData( | ||
Environment* env, | ||
const FunctionCallbackInfo<Value>& args, | ||
const int i) { | ||
if (!args[i]->IsObject()) { | ||
return false; | ||
} | ||
Local<Value> value = | ||
args[i].As<Object>()->Get(env->produce_cached_data_string()); | ||
|
||
return value->IsTrue(); | ||
} | ||
|
||
|
||
static Local<Integer> GetLineOffsetArg( | ||
const FunctionCallbackInfo<Value>& args, | ||
const int i) { | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are the factors that determine whether or not V8 will accept the cached data or not?
I'm trying to better understand it from reading the test cases below, but will V8 only accept the cached data if the script to be compiled is the same?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are several of them:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't that make it useless for release builds? The CPU features of the system the release binary is built on will be different from the systems it runs on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about this too, but I believe that there is much less CPU feature variety around us now. Especially on OS X.
We should definitely check this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure I agree. Take AVX: only newer Macbooks support that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bnoordhuis mmm... and what do you suggest?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CPU feature matching is a perennial problem in JVMs that support AOT compilation as well. Usually the solution for such AOT compilation is to produce the AOT code on the first invocation on a given machine and then persist it so that subsequent invocations can use the results of the first compilation. I am not sure the complexity is worth it in this case for Node, specially because the JVM class libraries tend to be much larger than the node core API, so AOT compilation has a bigger payoff on the JVMs.
This also doesn't help in scenarios where people run node inside containers where you don't expect repeated invocations of node.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ofrobots This API could be used to produce such caching data. I guess we can just strip down the second commit and land it without it. Perhaps, V8 could be improved a bit, though, and rely only on the features that were actually used in the code, or allow compiling with reduced set of CPU features and using it on CPU that actually have wider feature support.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would leave performance on the table. You would want to use all the CPU features available on the machine that the code is actually running on. Over and above the CPU feature set (the architecture), modern JIT compilers tune code to the exact micro-architecture the code is going to be running on. For example, the IBM Power 5 and IBM Power 6 were not very different on the surface from CPU feature set point of view, but are radically different micro-architectures. Code compiled with the Power 5 feature set works fine on Power 6 but you can get significant performance improvements by tweaking the instruction selection and scheduling if you knew the code is going to run on a Power 6.
Shipping code built in the lab will leave these performance opportunities on the table.