On 2015/08/25 18:12:09, Michael Starzinger wrote:
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?
I'm somewhat torn about this as well, and initially actually had the lookup
implemented separately. However, the separate implementation would share a
lot
of the same code, and, if done via hash table lookup, use the same data
structure as well. Furthermore, the difference between C++ runtime
functions to
JS builtin functions is no larger than the difference between C++ runtime
functions and inlined %_ functions. Common to all of them is that they can
be
addressed via % syntax. So I think grouping the three of them together is
alright.
But I can be convince otherwise, if you feel strongly about this.
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.
