Hi gentlemen!
Thank you again for your reviews.
This is a next approach (as of Patch Set 13) to the same problem. Again
this is
more of experimental work, than CL ready for submitting.
As in test/mjsunit/debug-liveedit-2.js scenario there is a script already
in
use, and a patch (in form of source_string.replace(pos, len, new_string) ).
First I invoke full compilation of both old and new versions of the script
(with
lazy-compile disabled). From a small spy code I collect information about
all
functions.
I choose function, which encloses the entire patch. There are 2 options:
1. replace code of this function (all existing instances immediately change
their behavior),
2. replace code of enclosing function (existing instances still behave like
before, and only new instances get updated behavior).
I choose between this 2 options based on number of parameters and layout of
outer scopes the function expects: if none of them changed, option #1,
otherwise
it is #2.
In the unit test the innermost function "Chooser" changes its body. However,
existing instances of "Chooser" may not be updated: now they gonna use "p"
argument from outer scope, but the data was not saved. "ChooseAnimal"
function
is updated instead.
However, a newly instantiated Chooser demonstrates an updated behavior.
An additional script object is created and non-updated functions are
relinked to
the old version of script.
Again, this is a pretty brutal and naive. However, I'd like to try to
develop
this concept.
I'm really interested what do you think about this.
Peter
http://codereview.chromium.org/546125/diff/4001/4002
File src/debug-delay.js (right):
http://codereview.chromium.org/546125/diff/4001/4002#newcode748
src/debug-delay.js:748: Debug.change_script_live = function(script, pos,
old_len, new_str) {
On 2010/01/24 09:52:13, pfeldman wrote:
I'd call it something like 'patch_script'. Also comment on what it
does would be
nice.
Done.
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) {
On 2010/01/24 21:22:44, Michail Naganov wrote:
Vectors (from utils.h) are preferred instead of arrays because they do
bounds
checking in debug mode.
Changed to FixedArray as Soren suggested.
http://codereview.chromium.org/546125/diff/4001/4003#newcode7904
src/runtime.cc:7904: SharedFunctionInfo** buffer, int buffer_size) {
On 2010/01/25 12:41:36, Søren Gjesse wrote:
I would suggest a FixedArray for this. See DebugReferencedBy in
runtime.cc.
Done.
http://codereview.chromium.org/546125/diff/4001/4003#newcode7921
src/runtime.cc:7921: // int start_position =
shared->function_token_position();
On 2010/01/25 12:41:36, Søren Gjesse wrote:
Code in comment.
Done.
http://codereview.chromium.org/546125/diff/4001/4003#newcode7936
src/runtime.cc:7936: static Object*
Runtime_GetOverlappingFunctionInfos(Arguments args) {
On 2010/01/24 09:52:13, pfeldman wrote:
It is strange that mutator has a Get* name. Also, debugger functions
are
generally called Runtime_Debug*.
Done.
http://codereview.chromium.org/546125/diff/4001/4003#newcode7939
src/runtime.cc:7939: CONVERT_CHECKED(JSValue, script, args[0]);
On 2010/01/25 12:41:36, Søren Gjesse wrote:
Use CONVERT_ARG_CHECKED to get script to be in a Handle.
There is a little problem that it seems to be some kind of wrapper
there.
http://codereview.chromium.org/546125/diff/4001/4003#newcode7942
src/runtime.cc:7942: CONVERT_CHECKED(String, new_str1, args[3]);
On 2010/01/25 12:41:36, Søren Gjesse wrote:
Use CONVERT_ARG_CHECKED to get new_str to be in a Handle.
Thanks
http://codereview.chromium.org/546125/diff/4001/4003#newcode7952
src/runtime.cc:7952: // No GC must occur here
On 2010/01/24 21:22:44, Michail Naganov wrote:
FYI, usually AssertNoAllocation auto variable is used to ensure that
no heap
allocations take place => GC can't occur.
Thanks
Done
http://codereview.chromium.org/546125/diff/4001/4003#newcode7956
src/runtime.cc:7956: delete[] buffer;
On 2010/01/25 09:01:49, Michail Naganov wrote:
An alternative (and widely used in V8) approach is to make 2 passes
and avoid
re-allocation (or even avoid temporary array). Consider
EnumerateCompiledFunctions and LogCompiledFunctions pair of functions
as an
example.
I am doing 2 passes. However, I hope that 1 pass will be enough with a
luck.
http://codereview.chromium.org/546125/diff/4001/4003#newcode7956
src/runtime.cc:7956: delete[] buffer;
On 2010/01/25 12:41:36, Søren Gjesse wrote:
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.
I hope I am doing something similar here.
http://codereview.chromium.org/546125/diff/4001/4003#newcode7960
src/runtime.cc:7960:
On 2010/01/24 21:22:44, Michail Naganov wrote:
Then, this allocation should appear in a separate block.
Done
http://codereview.chromium.org/546125/diff/4001/4003#newcode7982
src/runtime.cc:7982: if (shared->function_token_position() !=
RelocInfo::kNoPosition) {
On 2010/01/25 09:01:49, Michail Naganov wrote:
What if you move this logic into a method of SFI?
I have already moved it elsewhere :)
http://codereview.chromium.org/546125
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
