Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Streamline object operations through a better SlotMap interface #1533

Merged
merged 1 commit into from
Jul 22, 2024
Merged

Streamline object operations through a better SlotMap interface #1533

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions benchmarks/build.gradle
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading