Please take another look
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:
On 2015/07/08 08:32:53, Yang wrote:
Do you really need Number, Boolean, String, Symbol, Date and RegExp
objects?
Yes. Unfortunately the rest of the initialization sequence requires some
of these. I could selectively remove one or two, but this seemed more
consistent.
https://codereview.chromium.org/1213203007/diff/220001/src/bootstrapper.cc#newcode2075
src/bootstrapper.cc:2075: int i = Natives::GetDebuggerCount();
On 2015/07/08 08:32:53, Yang wrote:
I'd rename 'i' into 'js_builtins_script_index'.
Done.
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();
On 2015/07/08 08:32:54, Yang wrote:
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|.
Done.
https://codereview.chromium.org/1213203007/diff/220001/src/snapshot/serialize.cc#newcode1443
src/snapshot/serialize.cc:1443: context->set(Context::NEXT_CONTEXT_LINK,
On 2015/07/08 08:32:54, Yang wrote:
Some comment here would be great.
Done.
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
On 2015/07/08 08:32:54, Yang wrote:
... all *functions* are derived ...
Done.
https://codereview.chromium.org/1213203007/diff/220001/src/snapshot/serialize.cc#newcode1655
src/snapshot/serialize.cc:1655: Context* current =
JSFunction::cast(obj)->context();
On 2015/07/08 08:32:54, Yang wrote:
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.
Done.
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++) {
On 2015/07/08 08:32:54, Yang wrote:
Could you refactor this duplicate code into a helper function that
takes the
cache and the builtins count as arguments?
Done.
https://codereview.chromium.org/1213203007/
--
--
v8-dev mailing list
[email protected]
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 [email protected].
For more options, visit https://groups.google.com/d/optout.
