That makes sense Sundar; thank you for the great explanation! You mentioned that a weak-ref based mapping would be required to fix this. I am assuming that this would involve ScriptObject maintaining a weakref to a ScriptObjectMirror instance? You also said that this would bring added complexity that you would have to deal with. Can you elaborate on that a little bit?
I played around with the source in ScriptObject and ScriptObjectMirror a little bit, and added a weakref to a mirror instance inside ScriptObject. The weakref in the ScriptObject is initialized via the ScriptObjectMirror constructor. In ScriptObjectMirror#wrap, it looks for the mirror first, and returns it if it is not null. Otherwise it creates a new instance and sets the weakref to that instance. This seemed to fix the original issue I was facing. I ran the nashorn test suite and there weren't any failures after my changes. However, I am sure there are subtleties and nuances I am not taking into account, and so I'm curious what kind of issues could arise from my change. I know this is a complex issue, but I would like to learn more about it and see if I can take a stab at solving it. Thanks! Vivin On Tue, Dec 22, 2015 at 7:58 PM, Sundararajan Athijegannathan < sundararajan.athijegannat...@oracle.com> wrote: > Hi, > > Addressing your question on j.u.IdentityHashMap. I assume the rest of the > questions were addressed by Attila. > > * Assume that we implement === for ScriptObjectMirror to be (JVM) object > identity comparison of the *wrapped* underlying objects (as in your > attempted patch) rather than identity of ScriptObjectMirror objects, what > would happen? > > Script writer would see === being true for potentially two *different* > ScriptObjectMirror instances. Based on the mental model that === means > strict object identity, s/he then may try to use ScriptObjectMirror objects > in a java.util.IdentityHashMap! IdentityHashMap itself would use JVM > object identity -- so two different ScriptObjectMirror instances m1, m2 > wrapping *same* underlying script object "sobj" are different > for IdentityHashMap implementation. But, this breaks the assumption by > the script writer! s/he thinks m1 === m2 true => IdentityHashMap should > treat m1 and m2 are identical! That is not the case and so model breaks for > script authors. That is why I said, if at all we want to preserve script > object identity with ScriptObjectMirror instances, we'd need to implement > weakref based mirroring. > > -Sundar > > > On 12/23/2015 1:41 AM, Vivin Suresh Paliath wrote: > > Hi Sundar, > > Thank you for the explanation! I hope I am not being too obtuse - I'm just > trying to get a handle on how things work! > > Object identity is implemented strictly per ECMAScript spec - which says >> === evals to be evaluated true only if "same object". Why should we treat >> ScriptObjectMirrors as special for identity comparison > > > In my view, ScriptObjectMirrors should be treated the same only if it is > made abundantly clear that the JavaScript object you're working with in > Nashorn, may sometimes be a wrapped object and not the actual object > itself. The problem is that right now there is no easy way to discern that, > and anticipate the side effects that come with that distinction. With > respect to the ES spec, I agree that === should evaluate to true if the > objects are the same object, and that holds true for the most part even if > the objects are wrapped. But the problem arises when you try to compare a > property of an object (which is itself a JS object) with itself, where the > parent object happens to be foreign. That's when you end up in a situation > where obj === obj returns true, but obj.prop === obj.prop returns false. > The assumption I am making is invalid only if I *know* that they are > mirror objects, but as I mentioned before, there is no easy way to > determine that. I can see a case where obj.prop is actually a getter that > returns a new instance of an object each time, and here obj.prop === > obj.prop would return false, but that fact would be evident from the source > of the JS object itself, and wouldn't be due to how foreign script-objects > are implemented in Nashorn. > > For eg. think of using that in j.u.IdentityHashMap and expect it work! >> [=== returns true and so IdentityHashMap should work as expected -- but it >> won't because IdentityHashMap goes by JVM level object identity! > > > Perhaps I am not understanding this example correctly. Are you saying that > inserts into the IdentityHashMap wouldn't work as expected? But wouldn't > the only comparison that is being performed be == in Java, which would be > comparing the ScriptObjectMirror instances anyway (i.e., the same behavior > as now). I tried out the following with and without my changes and I got > the same output (size of the map is 3): > > engine.eval("function Foo(src) { this.src = src }; var e = { x: new > Foo(\"what\") };"); > > > ScriptContext c = new SimpleScriptContext(); > c.setBindings(engine.createBindings(), ScriptContext.ENGINE_SCOPE); > > c.getBindings(ScriptContext.ENGINE_SCOPE).putAll(engine.getBindings(ScriptContext.ENGINE_SCOPE)); > > c.getBindings(ScriptContext.ENGINE_SCOPE).put("ihm", new > IdentityHashMap<Object, Object>()); > engine.eval("ihm.put(e.x, 1); ihm.put(e.x, 2); ihm.put(e.x, 3); > print(ihm.size());", c); > > > The one discrepancy that would exist is if you perform an === comparison > in JavaScript against something from the map. > > You're creating multiple ScriptContexts and associate ENGINE_SCOPE >> Bindings to isolate. And yet you want objects from "isolated" worlds to be >> treated exactly like "current isolate world". > > > The new contexts I create know nothing about each other; they only "know" > (in the sense that the "global" runtime-library objects are available) the > objects from the main context (and only because I copy them over). Also, I > am not treating the object from the main isolated-world the same way as the > one in the current one; I am really only trying to compare references (and > that too against themselves). > > >> If you do not want that kind of multiple ECMAScript global scope >> isolation at all, why not load your library/framework scripts before >> loading the user scripts? If you want to avoid clash b/w different user >> scripts (i.e., globals in user's scripts for eg.), you could implement >> "require" like primitive such as the one here > > > This is actually exactly what I am doing, and I am using require-like > functionality as well. The problem arises when I try to dereference a > property of an object that is a foreign script-object. For example, let's > assume the default context of the script engine has an "enum" SomeEnum, > with the enum CONST. When I get a custom script, I create a new context > (using createBindings) and copy everything over. So now the new context has > a foreign object called SomeEnum. Now if I do SomeEnum === SomeEnum, I get > back true, but SomeEnum.CONST === SomeEnum.CONST returns false. Is this > what you were saying weak-refs would solve? > > If you do want isolation and still want singleton enum-like objects, why >> not implement those in Java and expose to scripts? These would then be real >> JVM singleton objects and therefore usual identity would work. > > > That would be a workaround, but some code that I insert are autogenerated > JS equivalents of POJOs. Enums are implemented using this > <https://github.com/vivin/enumjs> library, which gives you type-safe > enums in JavaScript as well. So I cannot assume upfront what kind of Java > enums are going to be used since they come from a different project > entirely. > > Based on what you said, I agree with you that === is probably the wrong > place to do this and it seems that the thing that really needs to be fixed > is ensuring that properties of foreign objects return the same mirror > instance whenever dereferenced. As I said before, I can't think of a case > where obj.prop === obj.prop would return false *unless* prop is a > calculated property that returns a new instance of some object each time. > Even if that were the case, I don't see how that behavior would be broken > if the Nashorn runtime ensured that we got back the same mirror instance. > Regardless, wouldn't it notice that prop actually delegates to a getter > function, and then run that? But in the case where prop is a reference to > an actual object, you would still have the expected behavior. I > understand that these foreign objects are wrapped, but I am questioning > whether someone really needs to *know* that they are, if they are writing > JS code on Nashorn. > > My original aim when I was doing this was to provide a single > script-engine instance so that I could take advantage of optimizations on > common code, while providing isolated execution contexts. Objects in the > engine's default context are "read-only" and the isolated contexts cannot > really mess with them. Really what I'm after is ensuring that library-code > gets optimized over time and that contexts can't step over each other. So > if such an approach gives me that, then it would definitely make things a > lot easier. Also, is there somewhere I can read about the optimizations > Nashorn makes in different cases? I'm specifically wondering about the > following: > > 1. Does Nashorn optimize pre-compiled JS code each time it is > evaluated? Are the optimizations persisted to the compiled representation? > 2. If I repeatedly evaluate the same JS code in the same > script-engine, but in different contexts, is that code optimized? Or are > the optimizations discarded once the context is gone? > > If I can still get these optimizations, then I can simply just precompile > and/or re-evaluate the library code in each new context and that would do > away with this issue and make things much easier. > > Thank you once again, and I apologize for so many questions! > > Vivin > > On Tue, Dec 22, 2015 at 1:35 AM, Sundararajan Athijegannathan < > <sundararajan.athijegannat...@oracle.com> > sundararajan.athijegannat...@oracle.com> wrote: > >> Hi, >> >> May be I should explain the background on how nashorn treates >> "foreign/host" objects. Nashorn implements object access primitives like >> "get_property, set_property, get_method, call, new" via a series of >> dynalink linkers ( <http://openjdk.java.net/jeps/276> >> http://openjdk.java.net/jeps/276). There is a linker for it's own >> objects (ScriptObject instances), there is one for Java objects ("beans >> linkers"), "foreign 'script' objects" (ScriptObjectMirrors). It allows even >> user specified linkers picked up via java.util.ServiceLoader mechanism. >> (See >> http://hg.openjdk.java.net/jdk9/dev/nashorn/file/74ddd1339c57/samples/dynalink >> ). >> >> But, looping (via for, for..each), identity and other operators are >> natively implemented in Nashorn. Object identity is implemented strictly >> per ECMAScript spec - which says === evals to be evaluated true only if >> "same object". Why should we treat ScriptObjectMirrors as special for >> identity comparison (and who knows what problems it could cause than the >> ones it solves!). For eg. think of using that in j.u.IdentityHashMap and >> expect it work! [=== returns true and so IdentityHashMap should work as >> expected -- but it won't because IdentityHashMap goes by JVM level object >> identity! As I said, it at all we want mirrors to preserve identity, we >> need weak refs. >> >> / >> >> Regarding host objects, even if they are different from native objects, >> it seems strange to me that the semantics of things like === would be >> different especially when doing something like obj.prop === obj.prop. The >> fact that such a statement could ever return false is non-obvious unless >> implementation details of the runtime are known. >> / >> >> The fact that you're assuming your host objects return "same object" for >> doing two "obj.prop" evals -- is an assumption about mirror objects! Why do >> you think such as assumption to be true for mirrors -- whose sole purpose >> is to provide easy access to objects. You're creating multiple >> ScriptContexts and associate ENGINE_SCOPE Bindings to isolate. And yet you >> want objects from "isolated" worlds to be treated exactly like "current >> isolate world". >> >> If you do not want that kind of multiple ECMAScript global scope >> isolation at all, why not load your library/framework scripts before >> loading the user scripts? If you want to avoid clash b/w different user >> scripts (i.e., globals in user's scripts for eg.), you could implement >> "require" like primitive such as the one here -> >> https://github.com/walterhiggins/commonjs-modules-javax-script >> >> If you do want isolation and still want singleton enum-like objects, why >> not implement those in Java and expose to scripts? These would then be real >> JVM singleton objects and therefore usual identity would work. >> >> Hope this helps, >> -Sundar >> >> >> On 12/22/2015 12:25 PM, Vivin Suresh Paliath wrote: >> >> I am looking at it from the perspective of someone writing code that is >> expect to run in JavaScript environment. I will describe the runtime >> environment I have set up. Perhaps what I am running into is an artifact of >> my approach. >> >> I have a single script engine instance. In this context I evaluate some >> JS source that populates the JS global scope with some objects. When a >> custom script needs to be evaluated, I create a new script context with a >> new global. Then I copy over all bindings from the parent script context >> into the new one. I also have a module system. When a custom script >> requests a core module, the module source is evaluated in the main script >> context and a single instance of the object exposed by the module source is >> cached. This way the source only needs to be evaluated once and benefits >> from optimizations. >> >> The problem I am running into has to do with some singleton objects I >> expose; kind of like enums, and can be compared using ===. I am using a JS >> library for this and this behavior holds in the browser and on NodeJS. On >> Nashorn, because the instances were created in a different context, === >> returns false. >> >> Is there a better way to accomplish what I am doing? >> >> Regarding host objects, even if they are different from native objects, >> it seems strange to me that the semantics of things like === would be >> different especially when doing something like obj.prop === obj.prop. The >> fact that such a statement could ever return false is non-obvious unless >> implementation details of the runtime are known. >> >> I am trying to expose my runtime environment as a JS environment where a >> developer can write custom scripts using some runtime libraries and >> utilities. So behavior like this would be very surprising, and requiring >> them to know that objects might be from different contexts seems like >> abstraction leakage from the way I have implemented the runtime. They >> shouldn't have to know whether an object is a wrapped ScriptObjectMirror >> instance, since from their point of view they are just working with JS >> objects, and they don't know that they are from different "worlds" since >> they don't even know that there are different worlds. >> >> I guess I am also having a hard time seeing a scenario where === would >> need to return false in the scenario I described, unless as a developer you >> knew there were different worlds. Even then, it would essentially make some >> forms of equality comparisons impossible (e.g., o.prop === o.prop if o is >> foreign). >> >> Does the snippet I posted earlier make sense? >> On Dec 21, 2015 9:10 PM, "Sundararajan Athijegannathan" < >> <sundararajan.athijegannat...@oracle.com> >> sundararajan.athijegannat...@oracle.com> wrote: >> >>> Actually, I'm not sure if depending on === in the code is a good >>> approach -- particularly, for objects that are not script objects of the >>> current world. These are to be treated like "host objects" in >>> ECMAScript-speak. i.e., regular rules of script objects don't always apply >>> to 'host objects'. The === operator for Java objects is interpreted as >>> object identity -- and the same rule for ScriptObjectMirrors -- in that >>> both are "host objects" from the standpoint of the 'current world'. >>> >>> -Sundar >>> >>> On 12/22/2015 9:34 AM, Vivin Suresh Paliath wrote: >>> >>> Thanks for the response! I understand that in general it would be >>> difficult for foreign objects. But this seems like surprising behavior >>> given that these are both JavaScript objects. That they are unequal seems >>> to be an artifact of the implementation (the fact that JS objects from >>> different contexts are treated as foreign). Could JS objects be treated >>> differently? >>> >>> After going through the Nashorn source, I decided to try this very naive >>> approach adding the following test to ScriptRuntime#equalSameTypeValues: >>> >>> if(x instanceof ScriptObjectMirror && y instanceof ScriptObjectMirror) { >>> return x.equals(y); >>> } >>> >>> After this change, the code now returns true, because >>> ScriptObjectMirror#equals compares the actual objects. I am not sure if >>> this breaks anything though (been trying to run the test suite, but end up >>> getting some errors from make). Is there a reason this particular fix is a >>> bad idea? I can't think of a particular reason why. From the perspective of >>> the runtime, I can't see a reason why those two objects should be >>> considered different. >>> >>> I will investigate the eval approach, but ideally I would like something >>> that doesn't impose any changes on the JS code. I am developing a simple >>> runtime that exposes some objects and utilities, and where custom scripts >>> can run in their own contexts. The fact that === returns false in these >>> cases leads to some very strange behavior. >>> >>> On Mon, Dec 21, 2015 at 8:48 PM, Sundararajan Athijegannathan < >>> <sundararajan.athijegannat...@oracle.com> >>> sundararajan.athijegannat...@oracle.com> wrote: >>> >>>> Unless we create mirrors as weak refs internally (i.e., maintain 1:1 >>>> with underlying foreign object reference), there is no easy solution. And >>>> maintaining such weak refs is unnecessarily complex. "Foreign object" >>>> call/access is mean to be just a "lightweight wrapper" based access. That >>>> said, you can do the === on the foreign context itself. You can call >>>> ScriptObjectMirror's eval to evaluate === test in that foreign context. >>>> That would get right object identity. >>>> >>>> -Sundar >>>> >>>> >>>> On 12/22/2015 4:26 AM, Vivin Suresh Paliath wrote: >>>> >>>>> One more thing I noticed is that apparently a new ScriptObjectMirror >>>>> instance is probably being created each time x is dereferenced, so >>>>> "e.x === >>>>> e.x" also returns "false". >>>>> >>>>> On Mon, Dec 21, 2015 at 3:49 PM, Vivin Suresh Paliath < >>>>> vivin.pali...@gmail.com> wrote: >>>>> >>>>> I ran into an issue where === returns false even when both should be >>>>>> pointing to the same object. I'm assuming this is because one of the >>>>>> objects is wrapped by a ScriptObjectMirror, because it was defined in >>>>>> a >>>>>> different context. >>>>>> >>>>>> Here's some code that demonstrates this: >>>>>> >>>>>> ScriptEngine engine = new >>>>>> NashornScriptEngineFactory().getScriptEngine( >>>>>> new String[] { "-strict" } >>>>>> ); >>>>>> >>>>>> try { >>>>>> engine.eval("function Foo(src) { this.src = src }; var e >>>>>> = { >>>>>> x: new Foo(\"what\") };"); >>>>>> >>>>>> ScriptContext c = new SimpleScriptContext(); >>>>>> c.setBindings(engine.createBindings(), >>>>>> ScriptContext.ENGINE_SCOPE); >>>>>> >>>>>> >>>>>> c.getBindings(ScriptContext.ENGINE_SCOPE).putAll(engine.getBindings(ScriptContext.ENGINE_SCOPE)); >>>>>> >>>>>> System.out.println(engine.eval("var z = e.x; z === >>>>>> e.x;", c)); >>>>>> } catch(Exception e) { >>>>>> throw new RuntimeException(e); >>>>>> } >>>>>> >>>>>> This prints out "false". Is there a way around this? I am also >>>>>> explicitly >>>>>> copying over all the bindings from the parent scope into the new >>>>>> scope so >>>>>> that I have access to "e". Could this be the source of the problem, >>>>>> and if >>>>>> so, is there a better way to achieve what I'm trying to do? >>>>>> >>>>>> -- >>>>>> Ruin untold; >>>>>> And thine own sadness, >>>>>> Sing in the grass, >>>>>> When eve has forgot, that no more hear common things that gleam and >>>>>> pass; >>>>>> But seek alone to lip, sad Rose of love and ruin untold; >>>>>> And thine own mother >>>>>> Can know it as I know >>>>>> More than another >>>>>> What makes your own sadness, >>>>>> Set in her eyes. >>>>>> >>>>>> map{@n=split//;$j.=$n[0]x$n[1]}split/:/,"01:11:02". >>>>>> ":11:01:11:02:13:01:11:01:11:01:13:02:12:01:13:01". >>>>>> ":11:04:11:06:12:04:11:01:12:01:13:02:12:01:14:01". >>>>>> ":13:01:11:03:12:01:11:04:12:02:11:01:11:01:13:02". >>>>>> ":11:03:11:06:11:01:11:05:12:02:11:01:11:01:13:02". >>>>>> ":11:02:12:01:12:04:11:06:12:01:11:04:12:04:11:01". >>>>>> ":12:03:12:01:12:01:11:01:12:01:12:02:11:01:11:01". >>>>>> ":13:02:11:01:02:11:01:12:02";map{print chr unpack" >>>>>> i",pack"B32",$_}$j=~m/.{8}/g >>>>>> >>>>>> >>>>> >>>>> >>>> >>> >>> >>> -- >>> Ruin untold; >>> And thine own sadness, >>> Sing in the grass, >>> When eve has forgot, that no more hear common things that gleam and pass; >>> But seek alone to lip, sad Rose of love and ruin untold; >>> And thine own mother >>> Can know it as I know >>> More than another >>> What makes your own sadness, >>> Set in her eyes. >>> >>> map{@n=split//;$j.=$n[0]x$n[1]}split/:/,"01:11:02". >>> ":11:01:11:02:13:01:11:01:11:01:13:02:12:01:13:01". >>> ":11:04:11:06:12:04:11:01:12:01:13:02:12:01:14:01". >>> ":13:01:11:03:12:01:11:04:12:02:11:01:11:01:13:02". >>> ":11:03:11:06:11:01:11:05:12:02:11:01:11:01:13:02". >>> ":11:02:12:01:12:04:11:06:12:01:11:04:12:04:11:01". >>> ":12:03:12:01:12:01:11:01:12:01:12:02:11:01:11:01". >>> ":13:02:11:01:02:11:01:12:02";map{print chr unpack" >>> i",pack"B32",$_}$j=~m/.{8}/g >>> >>> >>> >> > > > -- > Ruin untold; > And thine own sadness, > Sing in the grass, > When eve has forgot, that no more hear common things that gleam and pass; > But seek alone to lip, sad Rose of love and ruin untold; > And thine own mother > Can know it as I know > More than another > What makes your own sadness, > Set in her eyes. > > map{@n=split//;$j.=$n[0]x$n[1]}split/:/,"01:11:02". > ":11:01:11:02:13:01:11:01:11:01:13:02:12:01:13:01". > ":11:04:11:06:12:04:11:01:12:01:13:02:12:01:14:01". > ":13:01:11:03:12:01:11:04:12:02:11:01:11:01:13:02". > ":11:03:11:06:11:01:11:05:12:02:11:01:11:01:13:02". > ":11:02:12:01:12:04:11:06:12:01:11:04:12:04:11:01". > ":12:03:12:01:12:01:11:01:12:01:12:02:11:01:11:01". > ":13:02:11:01:02:11:01:12:02";map{print chr unpack" > i",pack"B32",$_}$j=~m/.{8}/g > > > -- Ruin untold; And thine own sadness, Sing in the grass, When eve has forgot, that no more hear common things that gleam and pass; But seek alone to lip, sad Rose of love and ruin untold; And thine own mother Can know it as I know More than another What makes your own sadness, Set in her eyes. map{@n=split//;$j.=$n[0]x$n[1]}split/:/,"01:11:02". ":11:01:11:02:13:01:11:01:11:01:13:02:12:01:13:01". ":11:04:11:06:12:04:11:01:12:01:13:02:12:01:14:01". ":13:01:11:03:12:01:11:04:12:02:11:01:11:01:13:02". ":11:03:11:06:11:01:11:05:12:02:11:01:11:01:13:02". ":11:02:12:01:12:04:11:06:12:01:11:04:12:04:11:01". ":12:03:12:01:12:01:11:01:12:01:12:02:11:01:11:01". ":13:02:11:01:02:11:01:12:02";map{print chr unpack" i",pack"B32",$_}$j=~m/.{8}/g