diff --git a/benchmarks/src/jmh/java/org/mozilla/javascript/benchmarks/SlotMapBenchmark.java b/benchmarks/src/jmh/java/org/mozilla/javascript/benchmarks/SlotMapBenchmark.java index a841daf131..43ee5955cd 100644 --- a/benchmarks/src/jmh/java/org/mozilla/javascript/benchmarks/SlotMapBenchmark.java +++ b/benchmarks/src/jmh/java/org/mozilla/javascript/benchmarks/SlotMapBenchmark.java @@ -44,7 +44,7 @@ public void create() { public Object embeddedInsert1Key(EmbeddedState state) { Slot newSlot = null; for (int i = 0; i < 100; i++) { - newSlot = state.emptyMap.modify(state.randomKeys[i], 0, 0); + newSlot = state.emptyMap.modify(null, state.randomKeys[i], 0, 0); } if (newSlot == null) { throw new AssertionError(); @@ -109,7 +109,7 @@ public void create() { public Object hashInsert1Key(HashState state) { Slot newSlot = null; for (int i = 0; i < 100; i++) { - newSlot = state.emptyMap.modify(state.randomKeys[i], 0, 0); + newSlot = state.emptyMap.modify(null, state.randomKeys[i], 0, 0); } if (newSlot == null) { throw new AssertionError(); @@ -156,7 +156,7 @@ private static String makeRandomString() { /** Insert a random key and value into the map */ private static String insertRandomEntry(SlotMap map) { String key = makeRandomString(); - Slot slot = map.modify(key, 0, 0); + Slot slot = map.modify(null, key, 0, 0); slot.setValue(key, null, null); return key; } diff --git a/rhino-xml/src/main/java/module-info.java b/rhino-xml/src/main/java/module-info.java index f88109a202..8ce70d199a 100644 --- a/rhino-xml/src/main/java/module-info.java +++ b/rhino-xml/src/main/java/module-info.java @@ -1,6 +1,9 @@ module org.mozilla.javascript.xml { exports org.mozilla.javascript.xmlimpl; + provides org.mozilla.javascript.xml.XMLLoader with + org.mozilla.javascript.xmlimpl.XMLLoaderImpl; + requires transitive org.mozilla.rhino; requires transitive java.xml; } diff --git a/rhino-xml/src/main/java/org/mozilla/javascript/xmlimpl/XMLLoaderImpl.java b/rhino-xml/src/main/java/org/mozilla/javascript/xmlimpl/XMLLoaderImpl.java new file mode 100644 index 0000000000..553153e280 --- /dev/null +++ b/rhino-xml/src/main/java/org/mozilla/javascript/xmlimpl/XMLLoaderImpl.java @@ -0,0 +1,22 @@ +package org.mozilla.javascript.xmlimpl; + +import org.mozilla.javascript.LazilyLoadedCtor; +import org.mozilla.javascript.ScriptableObject; +import org.mozilla.javascript.xml.XMLLib; +import org.mozilla.javascript.xml.XMLLoader; + +public class XMLLoaderImpl implements XMLLoader { + @Override + public void load(ScriptableObject scope, boolean sealed) { + String implClass = XMLLibImpl.class.getName(); + new LazilyLoadedCtor(scope, "XML", implClass, sealed, true); + new LazilyLoadedCtor(scope, "XMLList", implClass, sealed, true); + new LazilyLoadedCtor(scope, "Namespace", implClass, sealed, true); + new LazilyLoadedCtor(scope, "QName", implClass, sealed, true); + } + + @Override + public XMLLib.Factory getFactory() { + return XMLLib.Factory.create(XMLLibImpl.class.getName()); + } +} diff --git a/rhino-xml/src/main/resources/META-INF/services/org.mozilla.javascript.xml.XMLLoader b/rhino-xml/src/main/resources/META-INF/services/org.mozilla.javascript.xml.XMLLoader new file mode 100644 index 0000000000..48091e4ca6 --- /dev/null +++ b/rhino-xml/src/main/resources/META-INF/services/org.mozilla.javascript.xml.XMLLoader @@ -0,0 +1 @@ +org.mozilla.javascript.xmlimpl.XMLLoaderImpl diff --git a/rhino/src/main/java/module-info.java b/rhino/src/main/java/module-info.java index 1d33d575b7..32b36988d2 100644 --- a/rhino/src/main/java/module-info.java +++ b/rhino/src/main/java/module-info.java @@ -13,6 +13,12 @@ exports org.mozilla.javascript.typedarrays; exports org.mozilla.javascript.xml; + uses org.mozilla.javascript.RegExpProxy; + uses org.mozilla.javascript.xml.XMLLoader; + + provides org.mozilla.javascript.RegExpProxy with + org.mozilla.javascript.regexp.RegExpImpl; + requires java.compiler; requires jdk.dynalink; requires transitive java.desktop; diff --git a/rhino/src/main/java/org/mozilla/javascript/Context.java b/rhino/src/main/java/org/mozilla/javascript/Context.java index e397248502..48a0af3efa 100644 --- a/rhino/src/main/java/org/mozilla/javascript/Context.java +++ b/rhino/src/main/java/org/mozilla/javascript/Context.java @@ -35,6 +35,7 @@ import org.mozilla.javascript.debug.DebuggableScript; import org.mozilla.javascript.debug.Debugger; import org.mozilla.javascript.xml.XMLLib; +import org.mozilla.javascript.xml.XMLLoader; /** * This class represents the runtime context of an executing script. @@ -2318,11 +2319,19 @@ public boolean hasFeature(int featureIndex) { *

The default implementation uses the implementation provided by this Context's * {@link ContextFactory}. * + *

This is no longer used in E4X -- an implementation is only provided for backward + * compatibility. + * * @return An XMLLib.Factory. Should not return null if {@link #FEATURE_E4X} is * enabled. See {@link #hasFeature}. */ + @Deprecated public XMLLib.Factory getE4xImplementationFactory() { - return getFactory().getE4xImplementationFactory(); + XMLLoader loader = ScriptRuntime.loadOneServiceImplementation(XMLLoader.class); + if (loader != null) { + return loader.getFactory(); + } + return null; } /** @@ -2682,10 +2691,7 @@ private static boolean frameMatches(StackTraceElement e) { RegExpProxy getRegExpProxy() { if (regExpProxy == null) { - Class cl = Kit.classOrNull("org.mozilla.javascript.regexp.RegExpImpl"); - if (cl != null) { - regExpProxy = (RegExpProxy) Kit.newInstanceOrNull(cl); - } + regExpProxy = ScriptRuntime.loadOneServiceImplementation(RegExpProxy.class); } return regExpProxy; } diff --git a/rhino/src/main/java/org/mozilla/javascript/ContextFactory.java b/rhino/src/main/java/org/mozilla/javascript/ContextFactory.java index 8f5939f47e..43eef0fa4c 100644 --- a/rhino/src/main/java/org/mozilla/javascript/ContextFactory.java +++ b/rhino/src/main/java/org/mozilla/javascript/ContextFactory.java @@ -295,42 +295,6 @@ protected boolean hasFeature(Context cx, int featureIndex) { throw new IllegalArgumentException(String.valueOf(featureIndex)); } - private static boolean isDom3Present() { - Class nodeClass = Kit.classOrNull("org.w3c.dom.Node"); - if (nodeClass == null) return false; - // Check to see whether DOM3 is present; use a new method defined in - // DOM3 that is vital to our implementation - try { - nodeClass.getMethod("getUserData", String.class); - return true; - } catch (NoSuchMethodException e) { - return false; - } - } - - /** - * Provides a default {@link org.mozilla.javascript.xml.XMLLib.Factory XMLLib.Factory} to be - * used by the Context instances produced by this factory. See {@link - * Context#getE4xImplementationFactory} for details. - * - *

May return null, in which case E4X functionality is not supported in Rhino. - * - *

The default implementation now prefers the DOM3 E4X implementation. - */ - protected org.mozilla.javascript.xml.XMLLib.Factory getE4xImplementationFactory() { - // Must provide default implementation, rather than abstract method, - // so that past implementors of ContextFactory do not fail at runtime - // upon invocation of this method. - // Note that the default implementation returns null if we - // neither have XMLBeans nor a DOM3 implementation present. - - if (isDom3Present()) { - return org.mozilla.javascript.xml.XMLLib.Factory.create( - "org.mozilla.javascript.xmlimpl.XMLLibImpl"); - } - return null; - } - /** * Create class loader for generated classes. This method creates an instance of the default * implementation of {@link GeneratedClassLoader}. Rhino uses this interface to load generated diff --git a/rhino/src/main/java/org/mozilla/javascript/EmbeddedSlotMap.java b/rhino/src/main/java/org/mozilla/javascript/EmbeddedSlotMap.java index 0214341511..6be578ba70 100644 --- a/rhino/src/main/java/org/mozilla/javascript/EmbeddedSlotMap.java +++ b/rhino/src/main/java/org/mozilla/javascript/EmbeddedSlotMap.java @@ -94,7 +94,7 @@ public Slot query(Object key, int index) { * @param index index or 0 if slot holds property name. */ @Override - public Slot modify(Object key, int index, int attributes) { + public Slot modify(SlotMapOwner owner, Object key, int index, int attributes) { final int indexOrHash = (key != null ? key.hashCode() : index); Slot slot; @@ -110,13 +110,12 @@ public Slot modify(Object key, int index, int attributes) { } } - // A new slot has to be inserted. Slot newSlot = new Slot(key, index, attributes); - createNewSlot(newSlot); + createNewSlot(owner, newSlot); return newSlot; } - private void createNewSlot(Slot newSlot) { + private void createNewSlot(SlotMapOwner owner, Slot newSlot) { if (count == 0) { // Always throw away old slots if any on empty insert. slots = new Slot[INITIAL_SLOT_SIZE]; @@ -125,6 +124,11 @@ private void createNewSlot(Slot newSlot) { // Check if the table is not too full before inserting. if (4 * (count + 1) > 3 * slots.length) { // table size must be a power of 2 -- always grow by x2! + if (count > SlotMapContainer.LARGE_HASH_SIZE) { + var newMap = new HashSlotMap(this, newSlot); + owner.setMap(newMap); + return; + } Slot[] newSlots = new Slot[slots.length * 2]; copyTable(slots, newSlots); slots = newSlots; @@ -134,7 +138,8 @@ private void createNewSlot(Slot newSlot) { } @Override - public S compute(Object key, int index, SlotComputer c) { + public S compute( + SlotMapOwner owner, Object key, int index, SlotComputer c) { final int indexOrHash = (key != null ? key.hashCode() : index); if (slots != null) { @@ -148,54 +153,62 @@ public S compute(Object key, int index, SlotComputer c) { prev = slot; } if (slot != null) { - // Modify or remove existing slot - S newSlot = c.compute(key, index, slot); - if (newSlot == null) { - // Need to delete this slot actually - removeSlot(slot, prev, slotIndex, key); - } else if (!Objects.equals(slot, newSlot)) { - // Replace slot in hash table - if (prev == slot) { - slots[slotIndex] = newSlot; - } else { - prev.next = newSlot; - } - newSlot.next = slot.next; - // Replace new slot in linked list, keeping same order - if (slot == firstAdded) { - firstAdded = newSlot; - } else { - Slot ps = firstAdded; - while ((ps != null) && (ps.orderedNext != slot)) { - ps = ps.orderedNext; - } - if (ps != null) { - ps.orderedNext = newSlot; - } - } - newSlot.orderedNext = slot.orderedNext; - if (slot == lastAdded) { - lastAdded = newSlot; - } - } - return newSlot; + return computeExisting(key, index, c, slot, prev, slotIndex); } } + return computeNew(owner, key, index, c); + } - // If we get here, we know we are potentially adding a new slot + private S computeNew( + SlotMapOwner owner, Object key, int index, SlotComputer c) { S newSlot = c.compute(key, index, null); if (newSlot != null) { - createNewSlot(newSlot); + createNewSlot(owner, newSlot); + } + return newSlot; + } + + private S computeExisting( + Object key, int index, SlotComputer c, Slot slot, Slot prev, int slotIndex) { + // Modify or remove existing slot + S newSlot = c.compute(key, index, slot); + if (newSlot == null) { + // Need to delete this slot actually + removeSlot(slot, prev, slotIndex, key); + } else if (!Objects.equals(slot, newSlot)) { + // Replace slot in hash table + if (prev == slot) { + slots[slotIndex] = newSlot; + } else { + prev.next = newSlot; + } + newSlot.next = slot.next; + // Replace new slot in linked list, keeping same order + if (slot == firstAdded) { + firstAdded = newSlot; + } else { + Slot ps = firstAdded; + while ((ps != null) && (ps.orderedNext != slot)) { + ps = ps.orderedNext; + } + if (ps != null) { + ps.orderedNext = newSlot; + } + } + newSlot.orderedNext = slot.orderedNext; + if (slot == lastAdded) { + lastAdded = newSlot; + } } return newSlot; } @Override - public void add(Slot newSlot) { + public void add(SlotMapOwner owner, Slot newSlot) { if (slots == null) { slots = new Slot[INITIAL_SLOT_SIZE]; } - insertNewSlot(newSlot); + createNewSlot(owner, newSlot); } private void insertNewSlot(Slot newSlot) { diff --git a/rhino/src/main/java/org/mozilla/javascript/HashSlotMap.java b/rhino/src/main/java/org/mozilla/javascript/HashSlotMap.java index 542cf8e386..f4b4e308d1 100644 --- a/rhino/src/main/java/org/mozilla/javascript/HashSlotMap.java +++ b/rhino/src/main/java/org/mozilla/javascript/HashSlotMap.java @@ -26,10 +26,18 @@ public HashSlotMap() { public HashSlotMap(SlotMap oldMap) { map = new LinkedHashMap<>(oldMap.size()); for (Slot n : oldMap) { - add(n.copySlot()); + add(null, n.copySlot()); } } + public HashSlotMap(SlotMap oldMap, Slot newSlot) { + map = new LinkedHashMap<>(oldMap.dirtySize() + 1); + for (Slot n : oldMap) { + add(null, n.copySlot()); + } + add(null, newSlot); + } + @Override public int size() { return map.size(); @@ -47,21 +55,22 @@ public Slot query(Object key, int index) { } @Override - public Slot modify(Object key, int index, int attributes) { + public Slot modify(SlotMapOwner owner, Object key, int index, int attributes) { Object name = makeKey(key, index); return map.computeIfAbsent(name, n -> new Slot(key, index, attributes)); } @SuppressWarnings("unchecked") @Override - public S compute(Object key, int index, SlotComputer c) { + public S compute( + SlotMapOwner owner, Object key, int index, SlotComputer c) { Object name = makeKey(key, index); Slot ret = map.compute(name, (n, existing) -> c.compute(key, index, existing)); return (S) ret; } @Override - public void add(Slot newSlot) { + public void add(SlotMapOwner owner, Slot newSlot) { Object name = makeKey(newSlot); map.put(name, newSlot); } diff --git a/rhino/src/main/java/org/mozilla/javascript/ScriptRuntime.java b/rhino/src/main/java/org/mozilla/javascript/ScriptRuntime.java index 5b9bbf5012..d85d6e123b 100644 --- a/rhino/src/main/java/org/mozilla/javascript/ScriptRuntime.java +++ b/rhino/src/main/java/org/mozilla/javascript/ScriptRuntime.java @@ -14,15 +14,18 @@ import java.text.MessageFormat; import java.util.Arrays; import java.util.HashSet; +import java.util.Iterator; import java.util.Locale; import java.util.Optional; import java.util.ResourceBundle; +import java.util.ServiceLoader; import java.util.function.BiConsumer; import java.util.function.Consumer; import org.mozilla.javascript.ast.FunctionNode; import org.mozilla.javascript.v8dtoa.DoubleConversion; import org.mozilla.javascript.v8dtoa.FastDtoa; import org.mozilla.javascript.xml.XMLLib; +import org.mozilla.javascript.xml.XMLLoader; import org.mozilla.javascript.xml.XMLObject; /** @@ -194,19 +197,15 @@ public static ScriptableObject initSafeStandardObjects( NativeJavaObject.init(scope, sealed); NativeJavaMap.init(scope, sealed); - boolean withXml = - cx.hasFeature(Context.FEATURE_E4X) && cx.getE4xImplementationFactory() != null; - // define lazy-loaded properties using their class name new LazilyLoadedCtor( scope, "Continuation", "org.mozilla.javascript.NativeContinuation", sealed, true); - if (withXml) { - String xmlImpl = cx.getE4xImplementationFactory().getImplementationClassName(); - new LazilyLoadedCtor(scope, "XML", xmlImpl, sealed, true); - new LazilyLoadedCtor(scope, "XMLList", xmlImpl, sealed, true); - new LazilyLoadedCtor(scope, "Namespace", xmlImpl, sealed, true); - new LazilyLoadedCtor(scope, "QName", xmlImpl, sealed, true); + if (cx.hasFeature(Context.FEATURE_E4X)) { + XMLLoader loader = loadOneServiceImplementation(XMLLoader.class); + if (loader != null) { + loader.load(scope, sealed); + } } if (((cx.getLanguageVersion() >= Context.VERSION_1_8) @@ -5626,6 +5625,24 @@ public static void throwDeleteOnSuperPropertyNotAllowed() { throw referenceError("msg.delete.super"); } + /** + * Load a single implementation of "serviceClass" using the ServiceLoader. If there are no + * implementations, return null. If there is more than one implementation, throw a fatal + * exception, since this indicates that the classpath was configured incorrectly. + */ + static T loadOneServiceImplementation(Class serviceClass) { + Iterator it = ServiceLoader.load(serviceClass).iterator(); + if (it.hasNext()) { + T result = it.next(); + if (it.hasNext()) { + throw Kit.codeBug( + "Invalid configuration: more than one implementation of " + serviceClass); + } + return result; + } + return null; + } + public static final Object[] emptyArgs = new Object[0]; public static final String[] emptyStrings = new String[0]; } diff --git a/rhino/src/main/java/org/mozilla/javascript/ScriptableObject.java b/rhino/src/main/java/org/mozilla/javascript/ScriptableObject.java index 8d953ea28f..b747942c82 100644 --- a/rhino/src/main/java/org/mozilla/javascript/ScriptableObject.java +++ b/rhino/src/main/java/org/mozilla/javascript/ScriptableObject.java @@ -51,7 +51,7 @@ * @see org.mozilla.javascript.Scriptable * @author Norris Boyd */ -public abstract class ScriptableObject +public abstract class ScriptableObject extends SlotMapOwner implements Scriptable, SymbolScriptable, Serializable, DebuggableObject, ConstProperties { private static final long serialVersionUID = 2829861078851942586L; @@ -109,12 +109,6 @@ public abstract class ScriptableObject /** The parent scope of this object. */ private Scriptable parentScopeObject; - /** - * This holds all the slots. It may or may not be thread-safe, and may expand itself to a - * different data structure depending on the size of the object. - */ - private transient SlotMapContainer slotMap; - // Where external array data is stored. private transient ExternalArrayData externalData; @@ -157,24 +151,16 @@ static void checkValidAttributes(int attributes) { } } - private static SlotMapContainer createSlotMap(int initialSize) { - Context cx = Context.getCurrentContext(); - if ((cx != null) && cx.hasFeature(Context.FEATURE_THREAD_SAFE_OBJECTS)) { - return new ThreadSafeSlotMapContainer(initialSize); - } - return new SlotMapContainer(initialSize); - } - public ScriptableObject() { - slotMap = createSlotMap(0); + super(0); } public ScriptableObject(Scriptable scope, Scriptable prototype) { + super(0); if (scope == null) throw new IllegalArgumentException(); parentScopeObject = scope; prototypeObject = prototype; - slotMap = createSlotMap(0); } /** @@ -205,7 +191,7 @@ public String getTypeOf() { */ @Override public boolean has(String name, Scriptable start) { - return null != slotMap.query(name, 0); + return null != getMap().query(name, 0); } /** @@ -220,13 +206,13 @@ public boolean has(int index, Scriptable start) { if (externalData != null) { return (index < externalData.getArrayLength()); } - return null != slotMap.query(null, index); + return null != getMap().query(null, index); } /** A version of "has" that supports symbols. */ @Override public boolean has(Symbol key, Scriptable start) { - return null != slotMap.query(key, 0); + return null != getMap().query(key, 0); } /** @@ -240,7 +226,7 @@ public boolean has(Symbol key, Scriptable start) { */ @Override public Object get(String name, Scriptable start) { - Slot slot = slotMap.query(name, 0); + Slot slot = getMap().query(name, 0); if (slot == null) { return Scriptable.NOT_FOUND; } @@ -263,7 +249,7 @@ public Object get(int index, Scriptable start) { return Scriptable.NOT_FOUND; } - Slot slot = slotMap.query(null, index); + Slot slot = getMap().query(null, index); if (slot == null) { return Scriptable.NOT_FOUND; } @@ -273,7 +259,7 @@ public Object get(int index, Scriptable start) { /** Another version of Get that supports Symbol keyed properties. */ @Override public Object get(Symbol key, Scriptable start) { - Slot slot = slotMap.query(key, 0); + Slot slot = getMap().query(key, 0); if (slot == null) { return Scriptable.NOT_FOUND; } @@ -384,7 +370,7 @@ protected boolean putOwnProperty(Symbol key, Scriptable start, Object value, boo @Override public void delete(String name) { checkNotSealed(name, 0); - slotMap.compute(name, 0, ScriptableObject::checkSlotRemoval); + getMap().compute(this, name, 0, ScriptableObject::checkSlotRemoval); } /** @@ -397,14 +383,14 @@ public void delete(String name) { @Override public void delete(int index) { checkNotSealed(null, index); - slotMap.compute(null, index, ScriptableObject::checkSlotRemoval); + getMap().compute(this, null, index, ScriptableObject::checkSlotRemoval); } /** Removes an object like the others, but using a Symbol as the key. */ @Override public void delete(Symbol key) { checkNotSealed(key, 0); - slotMap.compute(key, 0, ScriptableObject::checkSlotRemoval); + getMap().compute(this, key, 0, ScriptableObject::checkSlotRemoval); } private static Slot checkSlotRemoval(Object key, int index, Slot slot) { @@ -458,7 +444,7 @@ public void defineConst(String name, Scriptable start) { */ @Override public boolean isConst(String name) { - Slot slot = slotMap.query(name, 0); + Slot slot = getMap().query(name, 0); if (slot == null) { return false; } @@ -561,7 +547,7 @@ public int getAttributes(Symbol sym) { */ public void setAttributes(String name, int attributes) { checkNotSealed(name, 0); - Slot attrSlot = slotMap.modify(name, 0, 0); + Slot attrSlot = getMap().modify(this, name, 0, 0); attrSlot.setAttributes(attributes); } @@ -579,14 +565,14 @@ public void setAttributes(String name, int attributes) { */ public void setAttributes(int index, int attributes) { checkNotSealed(null, index); - Slot attrSlot = slotMap.modify(null, index, 0); + Slot attrSlot = getMap().modify(this, null, index, 0); attrSlot.setAttributes(attributes); } /** Set attributes of a Symbol-keyed property. */ public void setAttributes(Symbol key, int attributes) { checkNotSealed(key, 0); - Slot attrSlot = slotMap.modify(key, 0, 0); + Slot attrSlot = getMap().modify(this, key, 0, 0); attrSlot.setAttributes(attributes); } @@ -599,9 +585,9 @@ public void setGetterOrSetter( AccessorSlot aSlot; if (isExtensible()) { // Create a new AccessorSlot, or cast it if it's already set - aSlot = slotMap.compute(name, index, ScriptableObject::ensureAccessorSlot); + aSlot = getMap().compute(this, name, index, ScriptableObject::ensureAccessorSlot); } else { - Slot slot = slotMap.query(name, index); + Slot slot = getMap().query(name, index); if (slot instanceof AccessorSlot) { aSlot = (AccessorSlot) slot; } else { @@ -647,7 +633,7 @@ public void setGetterOrSetter( */ public Object getGetterOrSetter(String name, int index, Scriptable scope, boolean isSetter) { if (name != null && index != 0) throw new IllegalArgumentException(name); - Slot slot = slotMap.query(name, index); + Slot slot = getMap().query(name, index); if (slot == null) return null; Function getterOrSetter = isSetter @@ -683,14 +669,14 @@ public Object getGetterOrSetter(String name, int index, boolean isSetter) { * @return whether the property is a getter or a setter */ protected boolean isGetterOrSetter(String name, int index, boolean setter) { - Slot slot = slotMap.query(name, index); + Slot slot = getMap().query(name, index); return (slot != null && slot.isSetterSlot()); } void addLazilyInitializedValue(String name, int index, LazilyLoadedCtor init, int attributes) { if (name != null && index != 0) throw new IllegalArgumentException(name); checkNotSealed(name, index); - LazyLoadSlot lslot = slotMap.compute(name, index, ScriptableObject::ensureLazySlot); + LazyLoadSlot lslot = getMap().compute(this, name, index, ScriptableObject::ensureLazySlot); lslot.setAttributes(attributes); lslot.value = init; } @@ -1588,7 +1574,8 @@ public void defineProperty( } } - AccessorSlot aSlot = slotMap.compute(propertyName, 0, ScriptableObject::ensureAccessorSlot); + AccessorSlot aSlot = + getMap().compute(this, propertyName, 0, ScriptableObject::ensureAccessorSlot); aSlot.setAttributes(attributes); if (getterBox != null) { aSlot.getter = new AccessorSlot.MemberBoxGetter(getterBox); @@ -1658,7 +1645,7 @@ protected boolean defineOwnProperty( } } - // this property lookup cannot happen from inside slotMap.compute lambda + // this property lookup cannot happen from inside getMap().compute lambda // as it risks causing a deadlock if ThreadSafeSlotMapContainer is used // and `this` is in prototype chain of `desc` Object enumerable = getProperty(desc, "enumerable"); @@ -1671,69 +1658,70 @@ protected boolean defineOwnProperty( // Do some complex stuff depending on whether or not the key // already exists in a single hash map operation - slotMap.compute( - key, - index, - (k, ix, existing) -> { - if (checkValid) { - checkPropertyChangeForSlot(id, existing, desc); - } - - Slot slot; - int attributes; - - if (existing == null) { - slot = new Slot(k, ix, 0); - attributes = - applyDescriptorToAttributeBitset( - DONTENUM | READONLY | PERMANENT, - enumerable, - writable, - configurable); - } else { - slot = existing; - attributes = - applyDescriptorToAttributeBitset( - existing.getAttributes(), - enumerable, - writable, - configurable); - } - - if (accessorDescriptor) { - AccessorSlot fslot; - if (slot instanceof AccessorSlot) { - fslot = (AccessorSlot) slot; - } else { - fslot = new AccessorSlot(slot); - slot = fslot; - } - if (getter != NOT_FOUND) { - fslot.getter = new AccessorSlot.FunctionGetter(getter); - } - - if (setter != NOT_FOUND) { - fslot.setter = new AccessorSlot.FunctionSetter(setter); - } - fslot.value = Undefined.instance; - } else { - if (!slot.isValueSlot() && isDataDescriptor(desc)) { - // Replace a non-base slot with a regular slot - slot = new Slot(slot); - } - - if (value != NOT_FOUND) { - slot.value = value; - } else if (existing == null) { - // Ensure we don't get a zombie value if we have switched a lot - slot.value = Undefined.instance; - } - } - - // After all that, whatever we return now ends up in the map - slot.setAttributes(attributes); - return slot; - }); + getMap().compute( + this, + key, + index, + (k, ix, existing) -> { + if (checkValid) { + checkPropertyChangeForSlot(id, existing, desc); + } + + Slot slot; + int attributes; + + if (existing == null) { + slot = new Slot(k, ix, 0); + attributes = + applyDescriptorToAttributeBitset( + DONTENUM | READONLY | PERMANENT, + enumerable, + writable, + configurable); + } else { + slot = existing; + attributes = + applyDescriptorToAttributeBitset( + existing.getAttributes(), + enumerable, + writable, + configurable); + } + + if (accessorDescriptor) { + AccessorSlot fslot; + if (slot instanceof AccessorSlot) { + fslot = (AccessorSlot) slot; + } else { + fslot = new AccessorSlot(slot); + slot = fslot; + } + if (getter != NOT_FOUND) { + fslot.getter = new AccessorSlot.FunctionGetter(getter); + } + + if (setter != NOT_FOUND) { + fslot.setter = new AccessorSlot.FunctionSetter(setter); + } + fslot.value = Undefined.instance; + } else { + if (!slot.isValueSlot() && isDataDescriptor(desc)) { + // Replace a non-base slot with a regular slot + slot = new Slot(slot); + } + + if (value != NOT_FOUND) { + slot.value = value; + } else if (existing == null) { + // Ensure we don't get a zombie value if we have switched a lot + slot.value = Undefined.instance; + } + } + + // After all that, whatever we return now ends up in the map + slot.setAttributes(attributes); + return slot; + }); return true; } @@ -1753,7 +1741,7 @@ protected boolean defineOwnProperty( */ public void defineProperty( String name, Supplier getter, Consumer setter, int attributes) { - LambdaSlot slot = slotMap.compute(name, 0, ScriptableObject::ensureLambdaSlot); + LambdaSlot slot = getMap().compute(this, name, 0, ScriptableObject::ensureLambdaSlot); slot.setAttributes(attributes); slot.getter = getter; slot.setter = setter; @@ -1789,19 +1777,20 @@ public void defineProperty( LambdaAccessorSlot newSlot = createLambdaAccessorSlot(name, 0, getter, setter, attributes); ScriptableObject newDesc = newSlot.buildPropertyDescriptor(cx); checkPropertyDefinition(newDesc); - slotMap.compute( - name, - 0, - (id, index, existing) -> { - if (existing != null) { - // it's dangerous to use `this` as scope inside slotMap.compute. - // It can cause deadlock when ThreadSafeSlotMapContainer is used - - return replaceExistingLambdaSlot(cx, name, existing, newSlot); - } - checkPropertyChangeForSlot(name, null, newDesc); - return newSlot; - }); + getMap().compute( + this, + name, + 0, + (id, index, existing) -> { + if (existing != null) { + // it's dangerous to use `this` as scope inside slotMap.compute. + // It can cause deadlock when ThreadSafeSlotMapContainer is used + + return replaceExistingLambdaSlot(cx, name, existing, newSlot); + } + checkPropertyChangeForSlot(name, null, newDesc); + return newSlot; + }); } private LambdaAccessorSlot replaceExistingLambdaSlot( @@ -2155,9 +2144,9 @@ public void sealObject() { } toInitialize.clear(); - long stamp = slotMap.readLock(); + long stamp = getMap().readLock(); try { - for (Slot slot : slotMap) { + for (Slot slot : getMap()) { Object value = slot.value; if (value instanceof LazilyLoadedCtor) { toInitialize.add(slot); @@ -2167,7 +2156,7 @@ public void sealObject() { isSealed = true; } } finally { - slotMap.unlockRead(stamp); + getMap().unlockRead(stamp); } } } @@ -2716,7 +2705,7 @@ private boolean putImpl( // so we inline the extensible/sealed checks below. Slot slot; if (this != start) { - slot = slotMap.query(key, index); + slot = getMap().query(key, index); if (!isExtensible && (slot == null || (!(slot instanceof AccessorSlot) @@ -2728,7 +2717,7 @@ private boolean putImpl( return false; } } else if (!isExtensible) { - slot = slotMap.query(key, index); + slot = getMap().query(key, index); if ((slot == null || (!(slot instanceof AccessorSlot) && (slot.getAttributes() & READONLY) != 0)) @@ -2742,7 +2731,7 @@ private boolean putImpl( if (isSealed) { checkNotSealed(key, index); } - slot = slotMap.modify(key, index, 0); + slot = getMap().modify(this, key, index, 0); } return slot.setValue(value, this, start, isThrow); } @@ -2768,19 +2757,19 @@ private boolean putConstImpl( } Slot slot; if (this != start) { - slot = slotMap.query(name, index); + slot = getMap().query(name, index); if (slot == null) { return false; } } else if (!isExtensible()) { - slot = slotMap.query(name, index); + slot = getMap().query(name, index); if (slot == null) { return true; } } else { checkNotSealed(name, index); // either const hoisted declaration or initialization - slot = slotMap.modify(name, index, CONST); + slot = getMap().modify(this, name, index, CONST); int attr = slot.getAttributes(); if ((attr & READONLY) == 0) throw Context.reportRuntimeErrorById("msg.var.redecl", name); @@ -2796,7 +2785,7 @@ private boolean putConstImpl( } private Slot getAttributeSlot(String name, int index) { - Slot slot = slotMap.query(name, index); + Slot slot = getMap().query(name, index); if (slot == null) { String str = (name != null ? name : Integer.toString(index)); throw Context.reportRuntimeErrorById("msg.prop.not.found", str); @@ -2805,7 +2794,7 @@ private Slot getAttributeSlot(String name, int index) { } private Slot getAttributeSlot(Symbol key) { - Slot slot = slotMap.query(key, 0); + Slot slot = getMap().query(key, 0); if (slot == null) { throw Context.reportRuntimeErrorById("msg.prop.not.found", key); } @@ -2824,20 +2813,20 @@ Object[] getIds(boolean getNonEnumerable, boolean getSymbols) { a[i] = Integer.valueOf(i); } } - if (slotMap.isEmpty()) { + if (getMap().isEmpty()) { return a; } int c = externalLen; - final long stamp = slotMap.readLock(); + final long stamp = getMap().readLock(); try { - for (Slot slot : slotMap) { + for (Slot slot : getMap()) { if ((getNonEnumerable || (slot.getAttributes() & DONTENUM) == 0) && (getSymbols || !(slot.name instanceof Symbol))) { if (c == externalLen) { // Special handling to combine external array with additional properties Object[] oldA = a; - a = new Object[slotMap.dirtySize() + externalLen]; + a = new Object[getMap().dirtySize() + externalLen]; if (oldA != null) { System.arraycopy(oldA, 0, a, 0, externalLen); } @@ -2846,7 +2835,7 @@ Object[] getIds(boolean getNonEnumerable, boolean getSymbols) { } } } finally { - slotMap.unlockRead(stamp); + getMap().unlockRead(stamp); } Object[] result; @@ -2901,19 +2890,19 @@ private static LambdaSlot ensureLambdaSlot(Object name, int index, Slot existing private void writeObject(ObjectOutputStream out) throws IOException { out.defaultWriteObject(); - final long stamp = slotMap.readLock(); + final long stamp = getMap().readLock(); try { - int objectsCount = slotMap.dirtySize(); + int objectsCount = getMap().dirtySize(); if (objectsCount == 0) { out.writeInt(0); } else { out.writeInt(objectsCount); - for (Slot slot : slotMap) { + for (Slot slot : getMap()) { out.writeObject(slot); } } } finally { - slotMap.unlockRead(stamp); + getMap().unlockRead(stamp); } } @@ -2921,10 +2910,10 @@ private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundE in.defaultReadObject(); int tableSize = in.readInt(); - slotMap = createSlotMap(tableSize); + setMap(createSlotMap(tableSize)); for (int i = 0; i < tableSize; i++) { Slot slot = (Slot) in.readObject(); - slotMap.add(slot); + getMap().add(this, slot); } } @@ -2936,24 +2925,24 @@ protected ScriptableObject getOwnPropertyDescriptor(Context cx, Object id) { protected Slot querySlot(Context cx, Object id) { if (id instanceof Symbol) { - return slotMap.query(id, 0); + return getMap().query(id, 0); } StringIdOrIndex s = ScriptRuntime.toStringIdOrIndex(id); if (s.stringId == null) { - return slotMap.query(null, s.index); + return getMap().query(null, s.index); } - return slotMap.query(s.stringId, 0); + return getMap().query(s.stringId, 0); } // Partial implementation of java.util.Map. See NativeObject for // a subclass that implements java.util.Map. public int size() { - return slotMap.size(); + return getMap().size(); } public boolean isEmpty() { - return slotMap.isEmpty(); + return getMap().isEmpty(); } public Object get(Object key) { diff --git a/rhino/src/main/java/org/mozilla/javascript/SlotMap.java b/rhino/src/main/java/org/mozilla/javascript/SlotMap.java index b614ae056e..47596bae6d 100644 --- a/rhino/src/main/java/org/mozilla/javascript/SlotMap.java +++ b/rhino/src/main/java/org/mozilla/javascript/SlotMap.java @@ -39,7 +39,7 @@ public interface SlotComputer { * slots will not be modified. * @return a Slot, which will be created anew if no such slot exists. */ - Slot modify(Object key, int index, int attributes); + Slot modify(SlotMapOwner owner, Object key, int index, int attributes); /** * Retrieve the slot at EITHER key or index, or return null if the slot cannot be found. @@ -58,11 +58,24 @@ public interface SlotComputer { * code and is more efficient than making multiple calls to this interface. In order to allow * use of multiple Slot subclasses, this function is templatized. */ - S compute(Object key, int index, SlotComputer compute); + S compute(SlotMapOwner owner, Object key, int index, SlotComputer compute); /** * Insert a new slot to the map. Both "name" and "indexOrHash" must be populated. Note that * ScriptableObject generally adds slots via the "modify" method. */ - void add(Slot newSlot); + void add(SlotMapOwner owner, Slot newSlot); + + default long readLock() { + // No locking in the default implementation + return 0L; + } + + default void unlockRead(long stamp) { + // No locking in the default implementation + } + + default int dirtySize() { + return size(); + } } diff --git a/rhino/src/main/java/org/mozilla/javascript/SlotMapContainer.java b/rhino/src/main/java/org/mozilla/javascript/SlotMapContainer.java index 40b3d032d1..377ff67cd8 100644 --- a/rhino/src/main/java/org/mozilla/javascript/SlotMapContainer.java +++ b/rhino/src/main/java/org/mozilla/javascript/SlotMapContainer.java @@ -8,23 +8,25 @@ import java.util.Collections; import java.util.Iterator; +import java.util.NoSuchElementException; +import java.util.Objects; /** * This class holds the various SlotMaps of various types, and knows how to atomically switch * between them when we need to so that we use the right data structure at the right time. */ -class SlotMapContainer implements SlotMap { +class SlotMapContainer extends SlotMapOwner implements SlotMap { /** * Once the object has this many properties in it, we will replace the EmbeddedSlotMap with * HashSlotMap. We can adjust this parameter to balance performance for typical objects versus * performance for huge objects with many collisions. */ - private static final int LARGE_HASH_SIZE = 2000; + static final int LARGE_HASH_SIZE = 2000; - private static final int DEFAULT_SIZE = 10; + static final int DEFAULT_SIZE = 10; - private static class EmptySlotMap implements SlotMap { + private static final class EmptySlotMap implements SlotMap { @Override public Iterator iterator() { @@ -42,8 +44,11 @@ public boolean isEmpty() { } @Override - public Slot modify(Object key, int index, int attributes) { - return null; + public Slot modify(SlotMapOwner owner, Object key, int index, int attributes) { + var newSlot = new Slot(key, index, attributes); + var map = new SingleEntrySlotMap(newSlot); + owner.setMap(map); + return newSlot; } @Override @@ -52,81 +57,184 @@ public Slot query(Object key, int index) { } @Override - public void add(Slot newSlot) { - throw new IllegalStateException(); + public void add(SlotMapOwner owner, Slot newSlot) { + if (newSlot != null) { + var map = new SingleEntrySlotMap(newSlot); + owner.setMap(map); + } + } + + @Override + public S compute( + SlotMapOwner owner, Object key, int index, SlotComputer c) { + var newSlot = c.compute(key, index, null); + if (newSlot != null) { + var map = new SingleEntrySlotMap(newSlot); + owner.setMap(map); + } + return newSlot; + } + } + + private static final class Iter implements Iterator { + private Slot next; + + Iter(Slot slot) { + next = slot; + } + + @Override + public boolean hasNext() { + return next != null; } @Override - public S compute(Object key, int index, SlotComputer compute) { - throw new IllegalStateException(); + public Slot next() { + Slot ret = next; + if (ret == null) { + throw new NoSuchElementException(); + } + next = next.orderedNext; + return ret; } } - private static EmptySlotMap EMPTY_SLOT_MAP = new EmptySlotMap(); + static final class SingleEntrySlotMap implements SlotMap { + + SingleEntrySlotMap(Slot slot) { + assert (slot != null); + this.slot = slot; + } + + private final Slot slot; + + @Override + public Iterator iterator() { + return new Iter(slot); + } - protected SlotMap map; + @Override + public int size() { + return 1; + } + + @Override + public boolean isEmpty() { + return false; + } + + @Override + public Slot modify(SlotMapOwner owner, Object key, int index, int attributes) { + final int indexOrHash = (key != null ? key.hashCode() : index); + + if (indexOrHash == slot.indexOrHash && Objects.equals(slot.name, key)) { + return slot; + } + Slot newSlot = new Slot(key, index, attributes); + add(owner, newSlot); + return newSlot; + } + + @Override + public Slot query(Object key, int index) { + final int indexOrHash = (key != null ? key.hashCode() : index); + + if (indexOrHash == slot.indexOrHash && Objects.equals(slot.name, key)) { + return slot; + } + return null; + } + + @Override + public void add(SlotMapOwner owner, Slot newSlot) { + if (owner == null) { + throw new IllegalStateException(); + } else { + var newMap = new EmbeddedSlotMap(); + owner.setMap(newMap); + newMap.add(owner, slot); + newMap.add(owner, newSlot); + } + } + + @Override + public S compute( + SlotMapOwner owner, Object key, int index, SlotComputer c) { + var newMap = new EmbeddedSlotMap(); + owner.setMap(newMap); + newMap.add(owner, slot); + return newMap.compute(owner, key, index, c); + } + } + + static SlotMap EMPTY_SLOT_MAP = new EmptySlotMap(); SlotMapContainer() { this(DEFAULT_SIZE); } SlotMapContainer(int initialSize) { + super(initialMap(initialSize)); + } + + private static SlotMap initialMap(int initialSize) { if (initialSize == 0) { - map = EMPTY_SLOT_MAP; + return EMPTY_SLOT_MAP; } else if (initialSize > LARGE_HASH_SIZE) { - map = new HashSlotMap(); + return new HashSlotMap(); } else { - map = new EmbeddedSlotMap(); + return new EmbeddedSlotMap(); } } @Override public int size() { - return map.size(); + return getMap().size(); } + @Override public int dirtySize() { - return map.size(); + return getMap().size(); } @Override public boolean isEmpty() { - return map.isEmpty(); + return getMap().isEmpty(); } @Override - public Slot modify(Object key, int index, int attributes) { - checkMapSize(); - return map.modify(key, index, attributes); + public Slot modify(SlotMapOwner owner, Object key, int index, int attributes) { + return getMap().modify(this, key, index, attributes); } @Override - public S compute(Object key, int index, SlotComputer c) { - checkMapSize(); - return map.compute(key, index, c); + public S compute( + SlotMapOwner owner, Object key, int index, SlotComputer c) { + return getMap().compute(this, key, index, c); } @Override public Slot query(Object key, int index) { - return map.query(key, index); + return getMap().query(key, index); } @Override - public void add(Slot newSlot) { - checkMapSize(); - map.add(newSlot); + public void add(SlotMapOwner owner, Slot newSlot) { + getMap().add(this, newSlot); } @Override public Iterator iterator() { - return map.iterator(); + return getMap().iterator(); } + @Override public long readLock() { // No locking in the default implementation return 0L; } + @Override public void unlockRead(long stamp) { // No locking in the default implementation } @@ -136,11 +244,12 @@ public void unlockRead(long stamp) { * map to a HashMap that is more robust against large numbers of hash collisions. */ protected void checkMapSize() { + var map = getMap(); if (map == EMPTY_SLOT_MAP) { - map = new EmbeddedSlotMap(); + setMap(new EmbeddedSlotMap()); } else if ((map instanceof EmbeddedSlotMap) && map.size() >= LARGE_HASH_SIZE) { SlotMap newMap = new HashSlotMap(map); - map = newMap; + setMap(newMap); } } } diff --git a/rhino/src/main/java/org/mozilla/javascript/SlotMapOwner.java b/rhino/src/main/java/org/mozilla/javascript/SlotMapOwner.java new file mode 100644 index 0000000000..0588df00ff --- /dev/null +++ b/rhino/src/main/java/org/mozilla/javascript/SlotMapOwner.java @@ -0,0 +1,44 @@ +package org.mozilla.javascript; + +public abstract class SlotMapOwner { + private static final long serialVersionUID = 1L; + + /** + * This holds all the slots. It may or may not be thread-safe, and may expand itself to a + * different data structure depending on the size of the object. + */ + private SlotMap slotMap; + + protected SlotMapOwner() { + slotMap = createSlotMap(0); + } + + protected SlotMapOwner(int capacity) { + slotMap = createSlotMap(capacity); + } + + protected SlotMapOwner(SlotMap map) { + slotMap = map; + } + + protected static SlotMap createSlotMap(int initialSize) { + Context cx = Context.getCurrentContext(); + if ((cx != null) && cx.hasFeature(Context.FEATURE_THREAD_SAFE_OBJECTS)) { + return new ThreadSafeSlotMapContainer(initialSize); + } else if (initialSize == 0) { + return SlotMapContainer.EMPTY_SLOT_MAP; + } else if (initialSize > SlotMapContainer.LARGE_HASH_SIZE) { + return new HashSlotMap(); + } else { + return new EmbeddedSlotMap(); + } + } + + final SlotMap getMap() { + return slotMap; + } + + final void setMap(SlotMap newMap) { + slotMap = newMap; + } +} diff --git a/rhino/src/main/java/org/mozilla/javascript/ThreadSafeSlotMapContainer.java b/rhino/src/main/java/org/mozilla/javascript/ThreadSafeSlotMapContainer.java index c2c8d7c3ae..d632cf7123 100644 --- a/rhino/src/main/java/org/mozilla/javascript/ThreadSafeSlotMapContainer.java +++ b/rhino/src/main/java/org/mozilla/javascript/ThreadSafeSlotMapContainer.java @@ -27,14 +27,14 @@ class ThreadSafeSlotMapContainer extends SlotMapContainer { @Override public int size() { long stamp = lock.tryOptimisticRead(); - int s = map.size(); + int s = getMap().size(); if (lock.validate(stamp)) { return s; } stamp = lock.readLock(); try { - return map.size(); + return getMap().size(); } finally { lock.unlockRead(stamp); } @@ -43,42 +43,41 @@ public int size() { @Override public int dirtySize() { assert lock.isReadLocked(); - return map.size(); + return getMap().size(); } @Override public boolean isEmpty() { long stamp = lock.tryOptimisticRead(); - boolean e = map.isEmpty(); + boolean e = getMap().isEmpty(); if (lock.validate(stamp)) { return e; } stamp = lock.readLock(); try { - return map.isEmpty(); + return getMap().isEmpty(); } finally { lock.unlockRead(stamp); } } @Override - public Slot modify(Object key, int index, int attributes) { + public Slot modify(SlotMapOwner owner, Object key, int index, int attributes) { final long stamp = lock.writeLock(); try { - checkMapSize(); - return map.modify(key, index, attributes); + return getMap().modify(this, key, index, attributes); } finally { lock.unlockWrite(stamp); } } @Override - public S compute(Object key, int index, SlotComputer c) { + public S compute( + SlotMapOwner owner, Object key, int index, SlotComputer c) { final long stamp = lock.writeLock(); try { - checkMapSize(); - return map.compute(key, index, c); + return getMap().compute(this, key, index, c); } finally { lock.unlockWrite(stamp); } @@ -87,25 +86,24 @@ public S compute(Object key, int index, SlotComputer c) { @Override public Slot query(Object key, int index) { long stamp = lock.tryOptimisticRead(); - Slot s = map.query(key, index); + Slot s = getMap().query(key, index); if (lock.validate(stamp)) { return s; } stamp = lock.readLock(); try { - return map.query(key, index); + return getMap().query(key, index); } finally { lock.unlockRead(stamp); } } @Override - public void add(Slot newSlot) { + public void add(SlotMapOwner owner, Slot newSlot) { final long stamp = lock.writeLock(); try { - checkMapSize(); - map.add(newSlot); + getMap().add(this, newSlot); } finally { lock.unlockWrite(stamp); } @@ -133,7 +131,7 @@ public void unlockRead(long stamp) { @Override public Iterator iterator() { assert lock.isReadLocked(); - return map.iterator(); + return getMap().iterator(); } /** diff --git a/rhino/src/main/java/org/mozilla/javascript/xml/XMLLoader.java b/rhino/src/main/java/org/mozilla/javascript/xml/XMLLoader.java new file mode 100644 index 0000000000..2afd54cd61 --- /dev/null +++ b/rhino/src/main/java/org/mozilla/javascript/xml/XMLLoader.java @@ -0,0 +1,10 @@ +package org.mozilla.javascript.xml; + +import org.mozilla.javascript.ScriptableObject; + +/** This interface is used to load the XML implementation using the ServiceLoader pattern. */ +public interface XMLLoader { + void load(ScriptableObject scope, boolean sealed); + + XMLLib.Factory getFactory(); +} diff --git a/rhino/src/main/resources/META-INF/services/org.mozilla.javascript.RegExpProxy b/rhino/src/main/resources/META-INF/services/org.mozilla.javascript.RegExpProxy new file mode 100644 index 0000000000..b1e8adbb57 --- /dev/null +++ b/rhino/src/main/resources/META-INF/services/org.mozilla.javascript.RegExpProxy @@ -0,0 +1 @@ +org.mozilla.javascript.regexp.RegExpImpl diff --git a/rhino/src/test/java/org/mozilla/javascript/SlotMapTest.java b/rhino/src/test/java/org/mozilla/javascript/SlotMapTest.java index 9394c3b77c..b321d2ea1f 100644 --- a/rhino/src/test/java/org/mozilla/javascript/SlotMapTest.java +++ b/rhino/src/test/java/org/mozilla/javascript/SlotMapTest.java @@ -2,12 +2,13 @@ import static org.junit.Assert.*; -import java.lang.reflect.InvocationTargetException; -import java.util.Arrays; import java.util.Collection; import java.util.HashSet; import java.util.Iterator; +import java.util.List; import java.util.Random; +import java.util.function.Supplier; +import java.util.stream.Collectors; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; @@ -17,139 +18,172 @@ public class SlotMapTest { // Random number generator with fixed seed to ensure repeatable tests private static final Random rand = new Random(0); - private final SlotMap map; + private static class TestScriptableObject extends ScriptableObject { - public SlotMapTest(Class mapClass) - throws IllegalAccessException, - InstantiationException, - IllegalArgumentException, - InvocationTargetException, - NoSuchMethodException, - SecurityException { - this.map = mapClass.getDeclaredConstructor().newInstance(); + public TestScriptableObject() { + super(); + } + + public String getClassName() { + return "foo"; + } + } + + private final ScriptableObject obj; + + // `SingleSlotMaps` will always have a slot, so we need to record + // the starting size of our slot map and take that into account + // when testing with one of those as the initial map. + private final int startingSize; + + public SlotMapTest(Supplier mapSupplier) { + this.obj = new TestScriptableObject(); + this.obj.setMap(mapSupplier.get()); + startingSize = this.obj.getMap().size(); } @Parameterized.Parameters public static Collection mapTypes() { - return Arrays.asList( - new Object[][] { - {EmbeddedSlotMap.class}, - {HashSlotMap.class}, - {SlotMapContainer.class}, - {ThreadSafeSlotMapContainer.class}, - }); + List> suppliers = + List.of( + () -> SlotMapContainer.EMPTY_SLOT_MAP, + () -> new SlotMapContainer.SingleEntrySlotMap(new Slot(new Object(), 0, 0)), + () -> new EmbeddedSlotMap(), + () -> new HashSlotMap(), + () -> new SlotMapContainer(), + () -> new ThreadSafeSlotMapContainer()); + return suppliers.stream().map(i -> new Object[] {i}).collect(Collectors.toList()); } @Test public void empty() { - assertEquals(0, map.size()); - assertTrue(map.isEmpty()); - assertNull(map.query("notfound", 0)); - assertNull(map.query(null, 123)); + if (startingSize == 0) { + assertEquals(0, obj.getMap().size()); + assertTrue(obj.getMap().isEmpty()); + assertNull(obj.getMap().query("notfound", 0)); + assertNull(obj.getMap().query(null, 123)); + } else { + assertEquals(startingSize, obj.getMap().size()); + assertFalse(obj.getMap().isEmpty()); + } } @Test public void crudOneString() { - assertNull(map.query("foo", 0)); - Slot slot = map.modify("foo", 0, 0); + assertNull(obj.getMap().query("foo", 0)); + Slot slot = obj.getMap().modify(obj, "foo", 0, 0); assertNotNull(slot); slot.value = "Testing"; - assertEquals(1, map.size()); - assertFalse(map.isEmpty()); + assertEquals(1 + startingSize, obj.getMap().size()); + assertFalse(obj.getMap().isEmpty()); Slot newSlot = new Slot(slot); - map.compute("foo", 0, (k, i, e) -> newSlot); - Slot foundNewSlot = map.query("foo", 0); + obj.getMap().compute(obj, "foo", 0, (k, i, e) -> newSlot); + Slot foundNewSlot = obj.getMap().query("foo", 0); assertEquals("Testing", foundNewSlot.value); assertSame(foundNewSlot, newSlot); - map.compute("foo", 0, (k, ii, e) -> null); - assertNull(map.query("foo", 0)); - assertEquals(0, map.size()); - assertTrue(map.isEmpty()); + obj.getMap().compute(obj, "foo", 0, (k, ii, e) -> null); + assertNull(obj.getMap().query("foo", 0)); + assertEquals(0 + startingSize, obj.getMap().size()); + if (startingSize == 0) { + assertTrue(obj.getMap().isEmpty()); + } else { + assertFalse(obj.getMap().isEmpty()); + } } @Test public void crudOneIndex() { - assertNull(map.query(null, 11)); - Slot slot = map.modify(null, 11, 0); + assertNull(obj.getMap().query(null, 11)); + Slot slot = obj.getMap().modify(obj, null, 11, 0); assertNotNull(slot); slot.value = "Testing"; - assertEquals(1, map.size()); - assertFalse(map.isEmpty()); + assertEquals(1 + startingSize, obj.getMap().size()); + assertFalse(obj.getMap().isEmpty()); Slot newSlot = new Slot(slot); - map.compute(null, 11, (k, i, e) -> newSlot); - Slot foundNewSlot = map.query(null, 11); + obj.getMap().compute(obj, null, 11, (k, i, e) -> newSlot); + Slot foundNewSlot = obj.getMap().query(null, 11); assertEquals("Testing", foundNewSlot.value); assertSame(foundNewSlot, newSlot); - map.compute(null, 11, (k, ii, e) -> null); - assertNull(map.query(null, 11)); - assertEquals(0, map.size()); - assertTrue(map.isEmpty()); + obj.getMap().compute(obj, null, 11, (k, ii, e) -> null); + assertNull(obj.getMap().query(null, 11)); + assertEquals(0 + startingSize, obj.getMap().size()); + if (startingSize == 0) { + assertTrue(obj.getMap().isEmpty()); + } else { + assertFalse(obj.getMap().isEmpty()); + } } @Test public void computeReplaceSlot() { - Slot slot = map.modify("one", 0, 0); + Slot slot = obj.getMap().modify(obj, "one", 0, 0); slot.value = "foo"; Slot newSlot = - map.compute( - "one", - 0, - (k, i, e) -> { - assertEquals(k, "one"); - assertEquals(i, 0); - assertNotNull(e); - assertEquals(e.value, "foo"); - Slot n = new Slot(e); - n.value = "bar"; - return n; - }); + obj.getMap() + .compute( + obj, + "one", + 0, + (k, i, e) -> { + assertEquals(k, "one"); + assertEquals(i, 0); + assertNotNull(e); + assertEquals(e.value, "foo"); + Slot n = new Slot(e); + n.value = "bar"; + return n; + }); assertEquals(newSlot.value, "bar"); - slot = map.query("one", 0); + slot = obj.getMap().query("one", 0); assertEquals(slot.value, "bar"); - assertEquals(map.size(), 1); + assertEquals(1 + startingSize, obj.getMap().size()); } @Test public void computeCreateNewSlot() { Slot newSlot = - map.compute( - "one", - 0, - (k, i, e) -> { - assertEquals(k, "one"); - assertEquals(i, 0); - assertNull(e); - Slot n = new Slot(k, i, 0); - n.value = "bar"; - return n; - }); + obj.getMap() + .compute( + obj, + "one", + 0, + (k, i, e) -> { + assertEquals(k, "one"); + assertEquals(i, 0); + assertNull(e); + Slot n = new Slot(k, i, 0); + n.value = "bar"; + return n; + }); assertNotNull(newSlot); assertEquals(newSlot.value, "bar"); - Slot slot = map.query("one", 0); + Slot slot = obj.getMap().query("one", 0); assertNotNull(slot); assertEquals(slot.value, "bar"); - assertEquals(map.size(), 1); + assertEquals(1 + startingSize, obj.getMap().size()); } @Test public void computeRemoveSlot() { - Slot slot = map.modify("one", 0, 0); + Slot slot = obj.getMap().modify(obj, "one", 0, 0); slot.value = "foo"; Slot newSlot = - map.compute( - "one", - 0, - (k, i, e) -> { - assertEquals(k, "one"); - assertEquals(i, 0); - assertNotNull(e); - assertEquals(e.value, "foo"); - return null; - }); + obj.getMap() + .compute( + obj, + "one", + 0, + (k, i, e) -> { + assertEquals(k, "one"); + assertEquals(i, 0); + assertNotNull(e); + assertEquals(e.value, "foo"); + return null; + }); assertNull(newSlot); - slot = map.query("one", 0); + slot = obj.getMap().query("one", 0); assertNull(slot); - assertEquals(map.size(), 0); + assertEquals(0 + startingSize, obj.getMap().size()); } private static final int NUM_INDICES = 67; @@ -157,29 +191,29 @@ public void computeRemoveSlot() { @Test public void manyKeysAndIndices() { for (int i = 0; i < NUM_INDICES; i++) { - Slot newSlot = map.modify(null, i, 0); + Slot newSlot = obj.getMap().modify(obj, null, i, 0); newSlot.value = i; } for (String key : KEYS) { - Slot newSlot = map.modify(key, 0, 0); + Slot newSlot = obj.getMap().modify(obj, key, 0, 0); newSlot.value = key; } - assertEquals(KEYS.length + NUM_INDICES, map.size()); - assertFalse(map.isEmpty()); + assertEquals(KEYS.length + NUM_INDICES + startingSize, obj.getMap().size()); + assertFalse(obj.getMap().isEmpty()); verifyIndicesAndKeys(); // Randomly replace some slots for (int i = 0; i < 20; i++) { int ix = rand.nextInt(NUM_INDICES); - Slot slot = map.query(null, ix); + Slot slot = obj.getMap().query(null, ix); assertNotNull(slot); - map.compute(null, ix, (k, j, e) -> new Slot(slot)); + obj.getMap().compute(obj, null, ix, (k, j, e) -> new Slot(slot)); } for (int i = 0; i < 20; i++) { int ix = rand.nextInt(KEYS.length); - Slot slot = map.query(KEYS[ix], 0); + Slot slot = obj.getMap().query(KEYS[ix], 0); assertNotNull(slot); - map.compute(KEYS[ix], 0, (k, j, e) -> new Slot(slot)); + obj.getMap().compute(obj, KEYS[ix], 0, (k, j, e) -> new Slot(slot)); } verifyIndicesAndKeys(); @@ -188,18 +222,18 @@ public void manyKeysAndIndices() { HashSet removedIds = new HashSet<>(); for (int i = 0; i < 20; i++) { int ix = rand.nextInt(NUM_INDICES); - map.compute(null, ix, (k, ii, e) -> null); + obj.getMap().compute(obj, null, ix, (k, ii, e) -> null); removedIds.add(ix); } HashSet removedKeys = new HashSet<>(); for (int i = 0; i < 20; i++) { int ix = rand.nextInt(NUM_INDICES); - map.compute(KEYS[ix], ix, (k, ii, e) -> null); + obj.getMap().compute(obj, KEYS[ix], ix, (k, ii, e) -> null); removedKeys.add(KEYS[ix]); } for (int i = 0; i < NUM_INDICES; i++) { - Slot slot = map.query(null, i); + Slot slot = obj.getMap().query(null, i); if (removedIds.contains(i)) { assertNull(slot); } else { @@ -208,7 +242,7 @@ public void manyKeysAndIndices() { } } for (String key : KEYS) { - Slot slot = map.query(key, 0); + Slot slot = obj.getMap().query(key, 0); if (removedKeys.contains(key)) { assertNull(slot); } else { @@ -220,20 +254,24 @@ public void manyKeysAndIndices() { private void verifyIndicesAndKeys() { long lockStamp = 0; - if (map instanceof SlotMapContainer) { - lockStamp = ((SlotMapContainer) map).readLock(); + if (obj.getMap() instanceof SlotMapContainer) { + lockStamp = ((SlotMapContainer) obj.getMap()).readLock(); } try { - Iterator it = map.iterator(); + Iterator it = obj.getMap().iterator(); + for (int i = 0; i < startingSize; i++) { + // Skip initial slots + it.next(); + } for (int i = 0; i < NUM_INDICES; i++) { - Slot slot = map.query(null, i); + Slot slot = obj.getMap().query(null, i); assertNotNull(slot); assertEquals(i, slot.value); assertTrue(it.hasNext()); assertEquals(slot, it.next()); } for (String key : KEYS) { - Slot slot = map.query(key, 0); + Slot slot = obj.getMap().query(key, 0); assertNotNull(slot); assertEquals(key, slot.value); assertTrue(it.hasNext()); @@ -241,14 +279,14 @@ private void verifyIndicesAndKeys() { } assertFalse(it.hasNext()); } finally { - if (map instanceof SlotMapContainer) { - ((SlotMapContainer) map).unlockRead(lockStamp); + if (obj.getMap() instanceof SlotMapContainer) { + ((SlotMapContainer) obj.getMap()).unlockRead(lockStamp); } } } // These keys come from the hash collision test and may help ensure that we have a few - // collisions for proper testing of the embedded slot map. + // collisions for proper testing of the embedded slot obj.getMap(). private static final String[] KEYS = { "AaAaAaAaAaAaAaAaAaAaAaAaAaAaAaAaAaAa", "AaAaAaAaAaAaAaAaAaAaAaAaAaAaAaAaAaBB",