This is an interesting aproach to changing the code of a running function.
However there are several issues which are somewhat troublesome. There are
bound
to be a large number of cornercases for a feature like this. Also there is
bound
to be some constraints to when this will be possible. I can think of the
following issues, but there are probably more.
1. Current activations of a function which is changed will stay the same,
and
cause a mix of code for that function. With our current code generation I
don't
think it will be possible to replace activated code. Should this be
disallowed
if the function is active?
2. What happens if the number of arguments to a function is changed? Call
sites
in other functions might be expecting the original number of arguments, and
will
set up adaptor frames for the original number of arguments.
3. With our current structure only function with a trivial scope can be
layily
compiled. See AllowsLazyCompilation() on FunctionLiteral. Functions which
does
not have these properties will not be suitable for this (this might change
at
some point though).
4. How about functions outside the patch itself. As far as I can see the
position in the script is updated in their SharedFunctionInfo, but if these
are
compiled then all the position information in the relocation information
should
also be patched.
5. What about existing script break-points? Some of these might also need
to be
moved.
6. If any of the constraints to allow this operation is violated an error
should
be returned and no change should happen.
Also there are quite a few long lines in this patch.
Another thing is that I am not sure whether this makes much sense if the
tool
performing the script patching is not controlling the actual source. E.g. if
changing source in the Chrome DevTools these changes will be transient as
the
source will be reloaded from some URL with no control from DevTools.
http://codereview.chromium.org/546125/diff/4001/4003
File src/runtime.cc (right):
http://codereview.chromium.org/546125/diff/4001/4003#newcode7904
src/runtime.cc:7904: SharedFunctionInfo** buffer, int buffer_size) {
I would suggest a FixedArray for this. See DebugReferencedBy in
runtime.cc.
http://codereview.chromium.org/546125/diff/4001/4003#newcode7921
src/runtime.cc:7921: // int start_position =
shared->function_token_position();
Code in comment.
http://codereview.chromium.org/546125/diff/4001/4003#newcode7939
src/runtime.cc:7939: CONVERT_CHECKED(JSValue, script, args[0]);
Use CONVERT_ARG_CHECKED to get script to be in a Handle.
http://codereview.chromium.org/546125/diff/4001/4003#newcode7942
src/runtime.cc:7942: CONVERT_CHECKED(String, new_str1, args[3]);
Use CONVERT_ARG_CHECKED to get new_str to be in a Handle.
http://codereview.chromium.org/546125/diff/4001/4003#newcode7956
src/runtime.cc:7956: delete[] buffer;
You can also take a look at Runtime_DebugReferencedBy and
DebugReferencedBy called by it. That is two iterations of the heap - one
for counting and one for filling a FixedArray.
http://codereview.chromium.org/546125
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
