Skip to content

Commit

Permalink
Streamline object operations through a better SlotMap interface
Browse files Browse the repository at this point in the history 
Implement a "compute" method which allows a bunch of the logic in
ScriptableObject work without as many slot map operations, especially
when changing slot instance types.

Also, get a particular check out of the slot map implementations that
has always bothered me and move it to ScriptableObject for better
encapsulation of language-specific behavior.
  • Loading branch information
@gbrail
gbrail committed Jul 22, 2024
1 parent c0e1768 commit b7ece52
Show file tree
Hide file tree
Showing 12 changed files with 315 additions and 262 deletions.
10 changes: 5 additions & 5 deletions benchmarks/build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ dependencies {

jmh {
// use this to include only some
//includes = ['V8']
//includes = ['SlotMap']
benchmarkMode = ['avgt']
fork = 1
timeUnit = 'us'
iterations = 5
//timeUnit = 'ns'
iterations = 3
timeOnIteration = '5s'
warmupIterations = 5
warmupIterations = 3
warmup = '5s'
}
}
4 changes: 4 additions & 0 deletions rhino/src/main/java/org/mozilla/javascript/AccessorSlot.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@
public class AccessorSlot extends Slot {
private static final long serialVersionUID = 1677840254177335827L;

AccessorSlot(Object name, int index) {
super(name, index, 0);
}

AccessorSlot(Slot oldSlot) {
super(oldSlot);
}
Expand Down
165 changes: 76 additions & 89 deletions rhino/src/main/java/org/mozilla/javascript/EmbeddedSlotMap.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,10 +111,12 @@ public Slot modify(Object key, int index, int attributes) {
}

// A new slot has to be inserted.
return createSlot(key, indexOrHash, attributes);
Slot newSlot = new Slot(key, index, attributes);
createNewSlot(newSlot);
return newSlot;
}

private Slot createSlot(Object key, int indexOrHash, int attributes) {
private void createNewSlot(Slot newSlot) {
if (count == 0) {
// Always throw away old slots if any on empty insert.
slots = new Slot[INITIAL_SLOT_SIZE];
Expand All @@ -128,52 +130,64 @@ private Slot createSlot(Object key, int indexOrHash, int attributes) {
slots = newSlots;
}

Slot newSlot = new Slot(key, indexOrHash, attributes);
insertNewSlot(newSlot);
return newSlot;
}

@Override
public void replace(Slot oldSlot, Slot newSlot) {
final int insertPos = getSlotIndex(slots.length, oldSlot.indexOrHash);
Slot prev = slots[insertPos];
Slot tmpSlot = prev;
// Find original slot and previous one in hash table
while (tmpSlot != null) {
if (tmpSlot == oldSlot) {
break;
}
prev = tmpSlot;
tmpSlot = tmpSlot.next;
}

// It's an error to call this when the slot isn't already there
assert (tmpSlot == oldSlot);

// add new slot to hash table
if (prev == oldSlot) {
slots[insertPos] = newSlot;
} else {
prev.next = newSlot;
}
newSlot.next = oldSlot.next;
public <S extends Slot> S compute(Object key, int index, SlotComputer<S> c) {
final int indexOrHash = (key != null ? key.hashCode() : index);

// Replace new slot in linked list, keeping same order
if (oldSlot == firstAdded) {
firstAdded = newSlot;
} else {
Slot ps = firstAdded;
while ((ps != null) && (ps.orderedNext != oldSlot)) {
ps = ps.orderedNext;
if (slots != null) {
Slot slot;
final int slotIndex = getSlotIndex(slots.length, indexOrHash);
Slot prev = slots[slotIndex];
for (slot = prev; slot != null; slot = slot.next) {
if (indexOrHash == slot.indexOrHash && Objects.equals(slot.name, key)) {
break;
}
prev = slot;
}
if (ps != null) {
ps.orderedNext = newSlot;
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;
}
}
newSlot.orderedNext = oldSlot.orderedNext;
if (oldSlot == lastAdded) {
lastAdded = newSlot;

// If we get here, we know we are potentially adding a new slot
S newSlot = c.compute(key, index, null);
if (newSlot != null) {
createNewSlot(newSlot);
}
return newSlot;
}

@Override
Expand All @@ -194,62 +208,35 @@ private void insertNewSlot(Slot newSlot) {
firstAdded = newSlot;
}
lastAdded = newSlot;
// add new slot to hash table, return it
addKnownAbsentSlot(slots, newSlot);
}

@Override
public void remove(Object key, int index) {
int indexOrHash = (key != null ? key.hashCode() : index);
private void removeSlot(Slot slot, Slot prev, int ix, Object key) {
count--;
// remove slot from hash table
if (prev == slot) {
slots[ix] = slot.next;
} else {
prev.next = slot.next;
}

if (count != 0) {
final int slotIndex = getSlotIndex(slots.length, indexOrHash);
Slot prev = slots[slotIndex];
Slot slot = prev;
while (slot != null) {
if (slot.indexOrHash == indexOrHash && Objects.equals(slot.name, key)) {
break;
}
prev = slot;
slot = slot.next;
}
if (slot != null) {
// non-configurable
if ((slot.getAttributes() & ScriptableObject.PERMANENT) != 0) {
Context cx = Context.getContext();
if (cx.isStrictMode()) {
throw ScriptRuntime.typeErrorById(
"msg.delete.property.with.configurable.false", key);
}
return;
}
count--;
// remove slot from hash table
if (prev == slot) {
slots[slotIndex] = slot.next;
} else {
prev.next = slot.next;
}
// remove from ordered list. Previously this was done lazily in
// getIds() but delete is an infrequent operation so O(n)
// should be ok

// remove from ordered list. Previously this was done lazily in
// getIds() but delete is an infrequent operation so O(n)
// should be ok

// ordered list always uses the actual slot
if (slot == firstAdded) {
prev = null;
firstAdded = slot.orderedNext;
} else {
prev = firstAdded;
while (prev.orderedNext != slot) {
prev = prev.orderedNext;
}
prev.orderedNext = slot.orderedNext;
}
if (slot == lastAdded) {
lastAdded = prev;
}
// ordered list always uses the actual slot
if (slot == firstAdded) {
prev = null;
firstAdded = slot.orderedNext;
} else {
prev = firstAdded;
while (prev.orderedNext != slot) {
prev = prev.orderedNext;
}
prev.orderedNext = slot.orderedNext;
}
if (slot == lastAdded) {
lastAdded = prev;
}
}

Expand Down
39 changes: 6 additions & 33 deletions rhino/src/main/java/org/mozilla/javascript/HashSlotMap.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,24 +38,15 @@ public Slot query(Object key, int index) {
@Override
public Slot modify(Object key, int index, int attributes) {
Object name = makeKey(key, index);
Slot slot = map.get(name);
if (slot != null) {
return slot;
}

return createSlot(key, index, attributes);
return map.computeIfAbsent(name, n -> new Slot(key, index, attributes));
}

@SuppressWarnings("unchecked")
@Override
public void replace(Slot oldSlot, Slot newSlot) {
Object name = makeKey(oldSlot);
map.put(name, newSlot);
}

private Slot createSlot(Object key, int index, int attributes) {
Slot newSlot = new Slot(key, index, attributes);
add(newSlot);
return newSlot;
public <S extends Slot> S compute(Object key, int index, SlotComputer<S> c) {
Object name = makeKey(key, index);
Slot ret = map.compute(name, (n, existing) -> c.compute(key, index, existing));
return (S) ret;
}

@Override
Expand All @@ -64,24 +55,6 @@ public void add(Slot newSlot) {
map.put(name, newSlot);
}

@Override
public void remove(Object key, int index) {
Object name = makeKey(key, index);
Slot slot = map.get(name);
if (slot != null) {
// non-configurable
if ((slot.getAttributes() & ScriptableObject.PERMANENT) != 0) {
Context cx = Context.getContext();
if (cx.isStrictMode()) {
throw ScriptRuntime.typeErrorById(
"msg.delete.property.with.configurable.false", key);
}
return;
}
map.remove(name);
}
}

@Override
public Iterator<Slot> iterator() {
return map.values().iterator();
Expand Down
4 changes: 4 additions & 0 deletions rhino/src/main/java/org/mozilla/javascript/LambdaSlot.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@
public class LambdaSlot extends Slot {
private static final long serialVersionUID = -3046681698806493052L;

LambdaSlot(Object name, int index) {
super(name, index, 0);
}

LambdaSlot(Slot oldSlot) {
super(oldSlot);
}
Expand Down
4 changes: 4 additions & 0 deletions rhino/src/main/java/org/mozilla/javascript/LazyLoadSlot.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@
* functions. It's used to load built-in objects more efficiently.
*/
public class LazyLoadSlot extends Slot {
LazyLoadSlot(Object name, int index) {
super(name, index, 0);
}

LazyLoadSlot(Slot oldSlot) {
super(oldSlot);
}
Expand Down
Loading

0 comments on commit b7ece52

Please sign in to comment.