LGTM, just a bunch of suggestions.
https://codereview.chromium.org/1289603002/diff/60001/src/api.cc
File src/api.cc (right):
https://codereview.chromium.org/1289603002/diff/60001/src/api.cc#newcode397
src/api.cc:397: }
I don't think any of this is actually necessary. What happens if you
remove this and the loop above for Natives?
https://codereview.chromium.org/1289603002/diff/60001/src/bootstrapper.cc
File src/bootstrapper.cc (right):
https://codereview.chromium.org/1289603002/diff/60001/src/bootstrapper.cc#newcode3231
src/bootstrapper.cc:3231: if (context_type != THIN_CONTEXT) {
I suggest putting InstallExtraNatives here. Then you don't need to pass
the context_type.
https://codereview.chromium.org/1289603002/diff/60001/src/snapshot/serialize.cc
File src/snapshot/serialize.cc (right):
https://codereview.chromium.org/1289603002/diff/60001/src/snapshot/serialize.cc#newcode581
src/snapshot/serialize.cc:581: Object* source =
isolate_->heap()->extra_natives_source_cache()->get(i);
now that we have this code three times, can we templatize it to reduce
duplicate code?
We have those GetCache templates in bootstrapper.cc, which we could move
to natives.h/natives.cc as methods of NativesCollection<>. We can then
use them here.
https://codereview.chromium.org/1289603002/diff/60001/src/snapshot/serialize.cc#newcode2208
src/snapshot/serialize.cc:2208:
serializer_->isolate()->heap()->extra_natives_source_cache(),
We can then also replace the third argument with
ExtraNatives::GetCache().
https://codereview.chromium.org/1289603002/diff/60001/src/snapshot/serialize.h
File src/snapshot/serialize.h (right):
https://codereview.chromium.org/1289603002/diff/60001/src/snapshot/serialize.h#newcode406
src/snapshot/serialize.h:406: static const int
kExtraNativesStringResource = 0x5e;
This is fine, but I'd group them a bit differently. Maybe 0x5d..0x5f for
the three k*StringResources, and sort it to right after kDeferred, swap
kVariableRepeat to 0x37 and leave 0x77 unused.
Not that it really matters, but it would be somewhat nicer.
https://codereview.chromium.org/1289603002/
--
--
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.
