Overall, great work. Really well thought out; adding features to runtimes to optimize special cases always has the possibility of becoming complicated, and you have managed to keep the changes to the minimum, and pretty well contained. E.g. I like how even before your changes we have a fast path in ScriptObject.findProperty for the case where the property is not in the prototype, so the shared proto check there only kicks in when we do actually need to deal with prototypes, etc.
I only have some very small remarks: - AllocatorMap could use getSharedPrototypeMap() in various places where it explicitly uses allocatorMap.getSharedProtoMap() now - Debug.scriptStack() doesn't seem to be used - PropertyMap.invalidateProtoGetSwitchPoint should be renamed invalidateProtoSwitchPoint. invalidateAllProtoGetSwitchPoints too - PropertyMap.invalidateSharedProtoSwitchPoint: there's the "switchPoint != null && !switchPoint.hasBeenInvalidated()" predicate. Is it possible for switchPoint to not be null and be invalidated? If not, maybe hasBeenInvalidated should be an assert within if instead - I wonder if "sharedProtoMap != myProto.getMap() || !myProto.getMap().isValidSharedProtoMap()" in ScriptObject.checkSharedProtoMap and a similar predicate "allocatorMap.getSharedProtoMap() != prototype.getMap() || !prototype.getMap().isValidSharedProtoMap())” in ScriptFunction.getAllocatorMap() couldn’t be extracted into a single method somewhere, if they share semantic similarity (syntactically they look very similar) Attila. > On Sep 11, 2015, at 2:20 PM, Hannes Wallnoefer <hannes.wallnoe...@oracle.com> > wrote: > > Please review JDK-8134609: Allow constructors with same prototoype map to > share the allocator map: > > http://cr.openjdk.java.net/~hannesw/8134609/webrev/ > > The details for this are a bit tricky, so I added some notes to the jira bug: > > https://bugs.openjdk.java.net/browse/JDK-8134609?focusedCommentId=13843176&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13843176 > > Thanks, > Hannes
