some comments.
https://codereview.chromium.org/1213203007/diff/220001/src/bootstrapper.cc
File src/bootstrapper.cc (right):
https://codereview.chromium.org/1213203007/diff/220001/src/bootstrapper.cc#newcode1106
src/bootstrapper.cc:1106:
Do you really need Number, Boolean, String, Symbol, Date and RegExp
objects?
https://codereview.chromium.org/1213203007/diff/220001/src/bootstrapper.cc#newcode2075
src/bootstrapper.cc:2075: int i = Natives::GetDebuggerCount();
I'd rename 'i' into 'js_builtins_script_index'.
https://codereview.chromium.org/1213203007/diff/220001/src/snapshot/serialize.cc
File src/snapshot/serialize.cc (right):
https://codereview.chromium.org/1213203007/diff/220001/src/snapshot/serialize.cc#newcode1197
src/snapshot/serialize.cc:1197: int index = source_.Get();
This duplicate code could be refactored into a helper function that
takes a Vector<const char>. Or you could handle both cases at once and
then choose the script source depending on the value of |data|.
https://codereview.chromium.org/1213203007/diff/220001/src/snapshot/serialize.cc#newcode1443
src/snapshot/serialize.cc:1443: context->set(Context::NEXT_CONTEXT_LINK,
Some comment here would be great.
https://codereview.chromium.org/1213203007/diff/220001/src/snapshot/serialize.cc#newcode1653
src/snapshot/serialize.cc:1653: // Make sure that all contexts are
derived from the code-stub context
... all *functions* are derived ...
https://codereview.chromium.org/1213203007/diff/220001/src/snapshot/serialize.cc#newcode1655
src/snapshot/serialize.cc:1655: Context* current =
JSFunction::cast(obj)->context();
Can you replace this with
DCHECK(!obj->IsJSFunction() || obj->GetCreationContext() ==
isolate()->heap()->code_stub_context())
There is a small difference between the creation context and the
function context, but the native context of the latter should be the
same. Either way you save yourself that explicit context walk.
https://codereview.chromium.org/1213203007/diff/220001/src/snapshot/serialize.cc#newcode2184
src/snapshot/serialize.cc:2184: for (int i = 0; i <
CodeStubNatives::GetBuiltinsCount(); i++) {
Could you refactor this duplicate code into a helper function that takes
the cache and the builtins count as arguments?
https://codereview.chromium.org/1213203007/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
