nodeprof.js icon indicating copy to clipboard operation
nodeprof.js copied to clipboard

for in/of support

Open aralisza opened this issue 5 years ago • 13 comments

My team is interested in instrumenting for...in and for...of loops? There was a forinObject callback in jalangi, but no callback for for...of loops since they weren't supported. Ideally, for our use case, we'd have an object read/write callback for every step of any loop, but having a forinObject and forofObject would be sufficient.

Are there any plans to support these types of for loops?

aralisza avatar Apr 07 '19 00:04 aralisza

Is there any update on this feature request? My team is hoping to use this feature soon.

mwaldrich avatar Jul 09 '19 19:07 mwaldrich

Sorry for the late reply! We are currently scoping how to provide the basic forinObject and forofObject callbacks in NodeProf.

alexjordan avatar Jul 19 '19 04:07 alexjordan

Any update on this feature?

mwaldrich avatar Aug 27 '19 20:08 mwaldrich

It's in progress. Currently, there is a workaround for for-of loop. We still need some support from Graal.js part for the for-in loop.

Haiyang-Sun avatar Aug 28 '19 18:08 Haiyang-Sun

Yes, the last missing bit is to be implemented on Graal.js -- I haven't had time to work on this recently, so perhaps @Haiyang-Sun can you prototype a patch to Graal.js and contribute that upstream directly (assuming you have time, of course) -- feel free to ping me on slack if you need guidance

eleinadani avatar Aug 28 '19 19:08 eleinadani

Ok, thanks for the work and the update!

mwaldrich avatar Aug 28 '19 22:08 mwaldrich

@eleinadani I found a workaround without changes to Graal.js. We simply keep track of the last expression's execution result that certainly would be the object being iterated. The patch and test example can be found in #45

@mwaldrich Please check if it works for you.

Haiyang-Sun avatar Aug 29 '19 15:08 Haiyang-Sun

Thanks for the implementation! We can now easily see the object involved with the loop.

My team was planning on tracking how data flows into the loop variable. However, I'm not sure this is possible with the current callbacks.

Here's an example.

Example

Code to instrument

var a = 1;
var b = [1, 2, a];
var z = 0;

for (var k of b) {
    z = k;
}

Analysis

(function (sandbox) {
    function MyAnalysis() {
        let lastExprResult;
        this.forObject = function (iid, isForIn) {
            console.log(`forObject isForIn=${isForIn}, object=${lastExprResult}`);
        };
        this.endExpression = function (iid, type, result) {
            lastExprResult = result;
        };
        /**
         * These callbacks are called after a variable is read or written.
         **/
        this.read = function (iid, name, val, isGlobal, isScriptLocal) {
            console.log(`read name=${name}, val=${val}`);
        };
        this.write = function (iid, name, val, lhs, isGlobal, isScriptLocal) {
            console.log(`write name=${name}, val=${val}`);
        };
        this.literal = function (iid, val, hasGetterSetter, literalType) {
            console.log(`literal val=${val}`);
            if (typeof val === "object") {
                console.log("object property names:");
                for (prop in val) {
                    if (val.hasOwnProperty(prop)) {
                        console.log("- " + prop);
                    }
                }
            }
        };
    }

    sandbox.analysis = new MyAnalysis();
})(J$);

NodeProf output

After running this with NodeProf, the output is:

literal val=function (exports, require, module, __filename, __dirname) {var a = 1;
var b = [1, 2, a]
var z = 0;

for (var k of b) {
    z = k;
}
}
literal val=1
write name=a, val=1
literal val=1
literal val=2
read name=a, val=1
literal val=1,2,1
object property names:
- 0
- 1
- 2
write name=b, val=1,2,1
literal val=0
write name=z, val=0
read name=b, val=1,2,1
forObject isForIn=false, object=1,2,1
write name=k, val=1
read name=k, val=1
write name=z, val=1
write name=k, val=2
read name=k, val=2
write name=z, val=2
write name=k, val=1
read name=k, val=1
write name=z, val=1

Question

I don't think it's possible with the current callbacks to know where the value of k comes from at any given time.

Consider a single loop iteration from the example above:

write name=k, val=1
read name=k, val=1
write name=z, val=1

These callbacks don't give you any information about where the value of k is coming from. These callbacks only tell you that it has been written to.

Proposal

There are a few ways to make this possible to track.

Emitting read callbacks for each iteration (preferred)

Perhaps the most simple solution I can think of is simply emitting the read callbacks for each iteration of the for loop. A single loop iteration would emit the callbacks:

getField base=b offset=0
write name=k, val=1
read name=k, val=1
write name=z, val=1

More for loop callbacks

Another possible way to track where the value of k comes from in each iteration is to add additional callbacks about for loops. For example, if we had callbacks for:

  • the start of a for loop
  • the end of a for loop
  • the start of a single for loop iteration

we would be able to maintain a stack of in-progress for loops in our analysis. At the beginning of a for loop iteration, we can index into the forObject at the top of the stack to get the value.

I would not prefer this solution though, as it would impose lots of overhead and complication on my team's analysis.

mwaldrich avatar Sep 17 '19 19:09 mwaldrich

https://github.com/Haiyang-Sun/nodeprof.js/pull/48 The second solution is easy and such a callback is anyway necessary to have. The first solution can be achieved similarly via adding a cfBlockEnter/Exit callback to report each iteration of the loop (the source section can be used to track the enclosing forIn/forOf node).

Haiyang-Sun avatar Sep 17 '19 20:09 Haiyang-Sun

I am confused about how the new callbacks could be used to instrument this. What callbacks would be generated in a single iteration of a for of loop?

Also, would it be possible for NodeProf to just generate the correct read callback when iterating through the loop? This would be my preferred solution, as it wouldn't require extra work in analyses to keep records about for loops.

mwaldrich avatar Sep 19 '19 20:09 mwaldrich

Regarding for-of loops, Graal.js does not expose the events we would need to provide a getField callback. I'm actually not certain that this could be provided given the implementation will be based on (possibly user-defined) iterators. We might be able to expose the iterator object in each iteration, but would this actually help?

alexjordan avatar Sep 23 '19 13:09 alexjordan

I'd like to separate issues for for-in and for-of and merge #48 if it sufficient to support for-in.

@Haiyang-Sun can you please update #48 based on my comments and rebase/squash the commit.

@mwaldrich is anything missing from #48 to support for-in?

alexjordan avatar Sep 26 '19 01:09 alexjordan

I proposed changes to the callbacks in #51. The instrumentation logic itself is unchanged.

alexjordan avatar Sep 30 '19 08:09 alexjordan