On Mon, Jan 25, 2010 at 3:41 PM, <[email protected]> wrote: > 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? > > Sounds like a valid limitation. Activated functions should not be changed (or changes to them should be scheduled).
> 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. > > It would work well in case of Eclipse debugger case though. You edit code on the fly and get it running right away. There is a set of scenarios where you would also want to have this functionality in DevTools. Especially if you want to affect the program's behavior in the middle of the complex user story (deep in the context). We can patch resource cache, so that reloads should not affect us badly. But as of this moment, it is just an over-the-weekend experiment that Peter wanted to make. > > 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
