Skip to content

Commit

Permalink
[GR-30727] Setting a new value should grow, set value, set shape in t…
Browse files Browse the repository at this point in the history 
…hat order.

PullRequest: graal/8769
  • Loading branch information
@woess
woess committed Apr 20, 2021
2 parents 31f6626 + b8d6ebc commit cef8f1a
Show file tree
Hide file tree
Showing 6 changed files with 87 additions and 47 deletions.
7 changes: 6 additions & 1 deletion truffle/src/com.oracle.truffle.object/src/com/oracle/truffle/object/CoreLocation.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2019, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2019, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* The Universal Permissive License (UPL), Version 1.0
Expand Down Expand Up @@ -67,6 +67,11 @@ protected final boolean isLongLocation() {
return this instanceof CoreLocations.LongLocation;
}

@Override
protected boolean isObjectLocation() {
return this instanceof CoreLocations.ObjectLocation;
}

/**
* Boxed values need to be compared by value not by reference.
*
Expand Down
26 changes: 20 additions & 6 deletions truffle/src/com.oracle.truffle.object/src/com/oracle/truffle/object/CoreLocations.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,9 @@ default void setInt(DynamicObject store, int value) throws FinalLocationExceptio

@Override
default void setInt(DynamicObject store, int value, Shape oldShape, Shape newShape) {
ACCESS.growAndSetShape(store, oldShape, newShape);
ACCESS.grow(store, oldShape, newShape);
setInt(store, value, false);
ACCESS.setShape(store, newShape);
}
}

Expand Down Expand Up @@ -146,8 +147,9 @@ default void setLong(DynamicObject store, long value) throws FinalLocationExcept

@Override
default void setLong(DynamicObject store, long value, Shape oldShape, Shape newShape) {
ACCESS.growAndSetShape(store, oldShape, newShape);
ACCESS.grow(store, oldShape, newShape);
setLong(store, value, false);
ACCESS.setShape(store, newShape);
}
}

Expand Down Expand Up @@ -181,8 +183,9 @@ default void setDouble(DynamicObject store, double value) throws FinalLocationEx

@Override
default void setDouble(DynamicObject store, double value, Shape oldShape, Shape newShape) {
ACCESS.growAndSetShape(store, oldShape, newShape);
ACCESS.grow(store, oldShape, newShape);
setDouble(store, value, false);
ACCESS.setShape(store, newShape);
}
}

Expand Down Expand Up @@ -214,8 +217,9 @@ default void setBoolean(DynamicObject store, boolean value) throws FinalLocation

@Override
default void setBoolean(DynamicObject store, boolean value, Shape oldShape, Shape newShape) {
ACCESS.growAndSetShape(store, oldShape, newShape);
ACCESS.grow(store, oldShape, newShape);
setBoolean(store, value, false);
ACCESS.setShape(store, newShape);
}
}

Expand Down Expand Up @@ -429,6 +433,11 @@ public final boolean isNonNull() {
return false;
}

@Override
protected void clear(DynamicObject store) {
setInternal(store, null, false);
}

@Override
public int objectArrayCount() {
return OBJECT_SLOT_SIZE;
Expand Down Expand Up @@ -466,6 +475,11 @@ public boolean isNonNull() {
return false;
}

@Override
protected void clear(DynamicObject store) {
setInternal(store, null, false);
}

@Override
public int objectFieldCount() {
return OBJECT_SLOT_SIZE;
Expand Down Expand Up @@ -998,9 +1012,9 @@ static int getLocationOrdinal(CoreLocation loc) {
LocationImpl internal = loc.getInternalLocation();
boolean isPrimitive = internal instanceof CoreLocations.LongLocation;
if (internal instanceof CoreLocations.FieldLocation) {
return (isPrimitive ? Integer.MIN_VALUE : 0) + ((CoreLocations.FieldLocation) internal).getIndex();
return (isPrimitive ? -Integer.MAX_VALUE : 0) + ((CoreLocations.FieldLocation) internal).getIndex();
} else if (internal instanceof CoreLocations.ArrayLocation) {
return (isPrimitive ? Integer.MIN_VALUE : 0) + MAX_DYNAMIC_FIELDS + ((CoreLocations.ArrayLocation) internal).getIndex();
return (isPrimitive ? -Integer.MAX_VALUE : 0) + MAX_DYNAMIC_FIELDS + ((CoreLocations.ArrayLocation) internal).getIndex();
} else {
throw new IllegalArgumentException(internal.getClass().getName());
}
Expand Down
71 changes: 49 additions & 22 deletions ...src/com.oracle.truffle.object/src/com/oracle/truffle/object/DynamicObjectLibraryImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -360,12 +360,13 @@ private static boolean putUncachedSlow(DynamicObject object, Object key, Object

assert ACCESS.getShape(object) == oldShape;
if (oldShape != newShape) {
ACCESS.growAndSetShape(object, oldShape, newShape);
ACCESS.grow(object, oldShape, newShape);
try {
getLocation(property).setInternal(object, value, false);
} catch (IncompatibleLocationException e) {
throw shouldNotHappen(e);
}
ACCESS.setShape(object, newShape);
updateShapeImpl(object);
} else {
try {
Expand All @@ -378,6 +379,7 @@ private static boolean putUncachedSlow(DynamicObject object, Object key, Object
}

static RemovePlan prepareRemove(ShapeImpl shapeBefore, ShapeImpl shapeAfter) {
assert !shapeBefore.isShared();
LayoutStrategy strategy = shapeBefore.getLayout().getStrategy();
List<Move> moves = new ArrayList<>();
boolean canMoveInPlace = shapeAfter.getObjectArrayCapacity() <= shapeBefore.getObjectArrayCapacity() &&
Expand Down Expand Up @@ -446,14 +448,8 @@ void performSet(DynamicObject obj, Object value) {
}

void clear(DynamicObject obj) {
if (fromLoc instanceof CoreLocations.ObjectLocation) {
// clear location to avoid memory leak
try {
fromLoc.setInternal(obj, null, false);
} catch (IncompatibleLocationException e) {
throw shouldNotHappen(e);
}
}
// clear location to avoid memory leak
fromLoc.clear(obj);
}

@Override
Expand Down Expand Up @@ -514,10 +510,11 @@ void perform(DynamicObject object) {
tempValues[i] = moves[i].performGet(object);
moves[i].clear(object);
}
ACCESS.resizeAndSetShape(object, shapeBefore, shapeAfter);
ACCESS.resize(object, shapeBefore, shapeAfter);
for (int i = moves.length - 1; i >= 0; i--) {
moves[i].performSet(object, tempValues[i]);
}
ACCESS.setShape(object, shapeAfter);
}
}

Expand Down Expand Up @@ -1242,7 +1239,7 @@ protected boolean putImpl(DynamicObject object, Shape cachedShape, Object key, O
if (location.canStore(value)) {
Shape newShape = c.newShape;
if (newShape != oldShape) {
ACCESS.growAndSetShape(object, oldShape, newShape);
ACCESS.grow(object, oldShape, newShape);
} else if (location.isFinal()) {
continue;
}
Expand All @@ -1251,6 +1248,9 @@ protected boolean putImpl(DynamicObject object, Shape cachedShape, Object key, O
} catch (IncompatibleLocationException | FinalLocationException e) {
throw shouldNotHappen(e);
}
if (newShape != oldShape) {
ACCESS.setShape(object, newShape);
}
c.maybeUpdateShape(object);
return true;
}
Expand Down Expand Up @@ -1283,7 +1283,7 @@ protected boolean putIntImpl(DynamicObject object, Shape cachedShape, Object key
boolean guardCondition = object.getShape() == oldShape;
if (location.isIntLocation()) {
if (newShape != oldShape) {
ACCESS.growAndSetShape(object, oldShape, newShape);
ACCESS.grow(object, oldShape, newShape);
} else if (location.isFinal()) {
continue;
}
Expand All @@ -1292,11 +1292,14 @@ protected boolean putIntImpl(DynamicObject object, Shape cachedShape, Object key
} catch (IncompatibleLocationException | FinalLocationException e) {
throw shouldNotHappen(e);
}
if (newShape != oldShape) {
ACCESS.setShape(object, newShape);
}
c.maybeUpdateShape(object);
return true;
} else if (location.isImplicitCastIntToLong()) {
if (newShape != oldShape) {
ACCESS.growAndSetShape(object, oldShape, newShape);
ACCESS.grow(object, oldShape, newShape);
} else if (location.isFinal()) {
continue;
}
Expand All @@ -1305,11 +1308,14 @@ protected boolean putIntImpl(DynamicObject object, Shape cachedShape, Object key
} catch (IncompatibleLocationException | FinalLocationException e) {
throw shouldNotHappen(e);
}
if (newShape != oldShape) {
ACCESS.setShape(object, newShape);
}
c.maybeUpdateShape(object);
return true;
} else if (location.isImplicitCastIntToDouble()) {
if (newShape != oldShape) {
ACCESS.growAndSetShape(object, oldShape, newShape);
ACCESS.grow(object, oldShape, newShape);
} else if (location.isFinal()) {
continue;
}
Expand All @@ -1318,11 +1324,14 @@ protected boolean putIntImpl(DynamicObject object, Shape cachedShape, Object key
} catch (IncompatibleLocationException | FinalLocationException e) {
throw shouldNotHappen(e);
}
if (newShape != oldShape) {
ACCESS.setShape(object, newShape);
}
c.maybeUpdateShape(object);
return true;
} else if (location.canStore(value)) {
if (newShape != oldShape) {
ACCESS.growAndSetShape(object, oldShape, newShape);
ACCESS.grow(object, oldShape, newShape);
} else if (location.isFinal()) {
continue;
}
Expand All @@ -1331,6 +1340,9 @@ protected boolean putIntImpl(DynamicObject object, Shape cachedShape, Object key
} catch (IncompatibleLocationException | FinalLocationException e) {
throw shouldNotHappen(e);
}
if (newShape != oldShape) {
ACCESS.setShape(object, newShape);
}
c.maybeUpdateShape(object);
return true;
}
Expand Down Expand Up @@ -1361,7 +1373,7 @@ protected boolean putLongImpl(DynamicObject object, Shape cachedShape, Object ke
if (location.isLongLocation()) {
Shape newShape = c.newShape;
if (newShape != oldShape) {
ACCESS.growAndSetShape(object, oldShape, newShape);
ACCESS.grow(object, oldShape, newShape);
} else if (location.isFinal()) {
continue;
}
Expand All @@ -1370,12 +1382,15 @@ protected boolean putLongImpl(DynamicObject object, Shape cachedShape, Object ke
} catch (IncompatibleLocationException | FinalLocationException e) {
throw shouldNotHappen(e);
}
if (newShape != oldShape) {
ACCESS.setShape(object, newShape);
}
c.maybeUpdateShape(object);
return true;
} else if (location.canStore(value)) {
Shape newShape = c.newShape;
if (newShape != oldShape) {
ACCESS.growAndSetShape(object, oldShape, newShape);
ACCESS.grow(object, oldShape, newShape);
} else if (location.isFinal()) {
continue;
}
Expand All @@ -1384,6 +1399,9 @@ protected boolean putLongImpl(DynamicObject object, Shape cachedShape, Object ke
} catch (IncompatibleLocationException | FinalLocationException e) {
throw shouldNotHappen(e);
}
if (newShape != oldShape) {
ACCESS.setShape(object, newShape);
}
c.maybeUpdateShape(object);
return true;
}
Expand Down Expand Up @@ -1414,7 +1432,7 @@ protected boolean putDoubleImpl(DynamicObject object, Shape cachedShape, Object
if (location.isDoubleLocation()) {
Shape newShape = c.newShape;
if (newShape != oldShape) {
ACCESS.growAndSetShape(object, oldShape, newShape);
ACCESS.grow(object, oldShape, newShape);
} else if (location.isFinal()) {
continue;
}
Expand All @@ -1423,12 +1441,15 @@ protected boolean putDoubleImpl(DynamicObject object, Shape cachedShape, Object
} catch (IncompatibleLocationException | FinalLocationException e) {
throw shouldNotHappen(e);
}
if (newShape != oldShape) {
ACCESS.setShape(object, newShape);
}
c.maybeUpdateShape(object);
return true;
} else if (newProperty.getLocation().canStore(value)) {
Shape newShape = c.newShape;
if (newShape != oldShape) {
ACCESS.growAndSetShape(object, oldShape, newShape);
ACCESS.grow(object, oldShape, newShape);
} else if (location.isFinal()) {
continue;
}
Expand All @@ -1437,6 +1458,9 @@ protected boolean putDoubleImpl(DynamicObject object, Shape cachedShape, Object
} catch (IncompatibleLocationException | FinalLocationException e) {
throw shouldNotHappen(e);
}
if (newShape != oldShape) {
ACCESS.setShape(object, newShape);
}
c.maybeUpdateShape(object);
return true;
}
Expand Down Expand Up @@ -1845,8 +1869,10 @@ abstract static class MakeSharedNode extends Node {
@Specialization
static void doCached(DynamicObject object, Shape cachedShape,
@Cached(value = "cachedShape.makeSharedShape()", allowUncached = true) Shape newShape) {
assert newShape != cachedShape;
ACCESS.growAndSetShape(object, cachedShape, newShape);
assert newShape != cachedShape &&
((ShapeImpl) cachedShape).getObjectArrayCapacity() == ((ShapeImpl) newShape).getObjectArrayCapacity() &&
((ShapeImpl) cachedShape).getPrimitiveArrayCapacity() == ((ShapeImpl) newShape).getPrimitiveArrayCapacity();
ACCESS.setShape(object, newShape);
}
}

Expand All @@ -1860,7 +1886,8 @@ static boolean doCached(DynamicObject object, Shape cachedShape, @SuppressWarnin
if (cachedShape == cachedOtherShape) {
return false;
}
ACCESS.resizeAndSetShape(object, cachedShape, cachedOtherShape);
ACCESS.resize(object, cachedShape, cachedOtherShape);
ACCESS.setShape(object, cachedOtherShape);
return true;
}

Expand Down
10 changes: 0 additions & 10 deletions ...fle/src/com.oracle.truffle.object/src/com/oracle/truffle/object/DynamicObjectSupport.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,16 +78,6 @@ static void trimToSize(DynamicObject object, Shape thisShape, Shape otherShape)
trimPrimitiveStore(object, thisShape, otherShape);
}

static void growAndSetShape(DynamicObject object, Shape thisShape, Shape otherShape) {
grow(object, thisShape, otherShape);
ACCESS.setShape(object, otherShape);
}

static void resizeAndSetShape(DynamicObject object, Shape thisShape, Shape otherShape) {
resize(object, thisShape, otherShape);
ACCESS.setShape(object, otherShape);
}

private static void growObjectStore(DynamicObject object, Shape oldShape, Shape newShape) {
int sourceCapacity = getObjectArrayCapacity(oldShape);
int destinationCapacity = getObjectArrayCapacity(newShape);
Expand Down
8 changes: 2 additions & 6 deletions truffle/src/com.oracle.truffle.object/src/com/oracle/truffle/object/LayoutImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,18 +171,14 @@ protected abstract static class Support extends Access {
protected Support() {
}

public final void growAndSetShape(DynamicObject object, Shape oldShape, Shape newShape) {
DynamicObjectSupport.growAndSetShape(object, oldShape, newShape);
public final void grow(DynamicObject object, Shape thisShape, Shape otherShape) {
DynamicObjectSupport.grow(object, thisShape, otherShape);
}

public final void resize(DynamicObject object, Shape thisShape, Shape otherShape) {
DynamicObjectSupport.resize(object, thisShape, otherShape);
}

public final void resizeAndSetShape(DynamicObject object, Shape thisShape, Shape otherShape) {
DynamicObjectSupport.resizeAndSetShape(object, thisShape, otherShape);
}

public final void invalidateAllPropertyAssumptions(Shape shape) {
DynamicObjectSupport.invalidateAllPropertyAssumptions(shape);
}
Expand Down
Loading

0 comments on commit cef8f1a

Please sign in to comment.