Adds createdAbstracts Set to effects/realm for tracking

[trueadm]

Release notes: adds createdAbstracts Set to both Effects and Realm.

For a feature I'm working on, where we can optionally inline function calls depending on a bunch of pre-defined heuristics, I've run into the need for a feature for tracking abstract values. When we use evaluateForEffects we currently get quite a bit of information of what occurred, nicely packaged up in effects. One of those is createdObjects, which is super useful for many cases, but I believe we also need, is a way of knowing what AbstractValues were created in effects too. This PR adds that functionality.

[sb98052]

Can you summarize what exactly you need this additional information for in the effects data structure?

[trueadm]

@sb98052 This additional information is used to known what abstract values are created in certain effects. For example, my other PR aims to wrap a function call in evaluateForEffects. We then take the Value that was returned from the function call's effects and apply some heuristics to determine if we should "inline" the function call and its effects, or instead leave the function call in code. When trying to determine if something should optionally be inlined, we have an odd case with abstract values where we do not know where if the abstract was created inside the function call, or if the abstract was created previously.

var someAbstractValueCreatedOutside = __abstract();

function foo(x, y) {
  return [x + y, someAbstractValueCreatedOutside];
}

var returnValueFromFunctionCall = foo(10, 2);

If you look at this example, we can avoid the function call altogether and avoid potential code bloat because we know that the function returns something that we can clone and re-model without the effects of the function (as long as we have effects.createdAbstracts to determine that someAbstractValueCreatedOutside was indeed not created in the function call.

[sb98052]

Ok, thanks. So you are saying that you have a heuristic for deciding whether a function should be inlined or outlined, and that heuristic relies on whether abstract values reachable from the return value of the function were created inside or referenced from the outside? Is that the totality of the heuristic?

I'm drilling in here because I ran into very similar issues when dealing with the effects of array operators, which depend on whether objects were created inside or outside. Also, to my mind effects track state that is mutable - such as property and binding locations, which AbstractValues are not.

So bear with me a bit longer :-)

What if an abstract value is created inside, but all of its args refer to pre-existing values? E.g. (outside boolean)?(outside object 1):(outside object 2). Then does the function not fall into the outlinable category? If so, then the test isn't "if an abstract value was created." Rather "if external locations are reachable from the abstract value." And this is what I implemented for the array case.

[trueadm]

@sb98052 The logic I have traverses through all args on abstract values, only if the abstract value exists in the effects. If all args of an abstract value are also all created in the function then we know it's not safe to re-model. If an abstract value was created in a function, but it's effects were not, we clone the abstract value outside of the function's effects and re-model it's args to be that of what were outside.

[sb98052]

If all args of an abstract value are also all created in the function then we know it's not safe to re-model

Did you mean "if any of the args of an abstract value are created in the function?"

There are two types of abstract values that get created - ones that are reachable via the return value, and ones that involve mutations to non-local state. Assuming that the functions you are dealing with are pure, it's not clear to me why the check does not amount to "are created objects reachable from the return value."

[trueadm]

@sb98052 Yes, I mean "any" sorry.

[cblappert]

Also, to my mind effects track state that is mutable - such as property and binding locations, which AbstractValues are not.

I think of Effects more as a summary of the function because it also includes the return value and set of objects created.

[sb98052]

Right, it is a summary that consists of two things: the functional part, i.e. the return value, and the side effects part. I was referring to the side effects part when I said "tracks mutable locations." It appears to me that for pure functions, abstract values go into the functional part - because they don't reflect a function's behavior, but rather how we model that behavior. It might be that the information needed is computable from the return value, as here:

https://github.com/facebook/prepack/blob/b942cd9635ee36787e1ea878616714ade7380799/src/values/ArrayValue.js#L146-L148

I should put up a PR shortly that cleans the API up.

[trueadm]

@sb98052 How can your logic know if an abstract value is a temporal created inside a function's effects vs a temporal created outside? I also use the effects.createdAbstracts to fast-path optimizations, so if I know something was created outside then I don't traverse through it's args and I don't need to clone/remodel it either.

[sb98052]

@trueadm You have a valid point. My logic (even the modified version I'm going to submit) cannot determine whether temporals reachable from the return value were created within the function or passed from the outside. I wonder though if what we need is a "temporal interactions" field that tracks temporals, rather than all abstract values. Those certainly fall into the "side effects" category since they summarize an aspect of side-effectful behavior. And it pertains to the analyzed code, rather than to our modeling constructs.

One thing that will happen if you track all abstract values is that since abstract values are immutable, every time one is transformed, it will show up in createdAbstracts. So if you have something like if (foo == null) and the simplifier expands it, you'll get three new abstract values. These will include values that are not reachable from the return value of the function. Again, if all you want to check is whether something in there is temporal, then it should work - but the field might end up accumulating a lot of clutter.

[trueadm]

@sb98052 That's a point, maybe createdAbstracts could be createdDerivedAbstracts instead. Food for thought.

[trueadm]

Merging this for now, just to unblock my PR. Will revise in a later PR.