Essentially one high-level comment/suggestion. Happy to discuss in person
and/or
be convinced otherwise. Would also be interested in what Benedikt thinks
about
this.
https://codereview.chromium.org/1306993003/diff/1/src/ast.h
File src/ast.h (right):
https://codereview.chromium.org/1306993003/diff/1/src/ast.h#newcode2052
src/ast.h:2052: const Runtime::Function* function_;
If below comment is addressed then we would need two fields here again,
either Runtime::Function or Context::Index. Still a memory saving from
what was here before, and if size really becomes the argument here, then
there are (other) ways to compress it down I think.
https://codereview.chromium.org/1306993003/diff/1/src/runtime/runtime.h
File src/runtime/runtime.h (right):
https://codereview.chromium.org/1306993003/diff/1/src/runtime/runtime.h#newcode995
src/runtime/runtime.h:995: enum IntrinsicType { RUNTIME, INLINE, CONTEXT
};
High-level feedback: I am not sure whether overloading the
Runtime::Function table like this is the way to go. Basically what we
need is a mapping (name -> context-index) IIUC. The other fields (e.g.
nargs, result_size) make no sense for "imported context functions". Also
them being implemented in JavaScript as opposed to C++ makes them
different. Would it be worth it to have a separate lookup functionality
outside of the Runtime class?
https://codereview.chromium.org/1306993003/
--
--
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.
