Skip to content

Commit 850bd4a

Browse files
woesswoess
committed
Revise PutNode implementation.
* Remove cached mode; check if the property flags match the expected flags according to mode. * Check oldShape valid assumption in specialization and update shape if newShape is obsolete.
1 parent 754b643 commit 850bd4a

File tree

1 file changed

+61
-54
lines changed
  • truffle/src/com.oracle.truffle.api.object/src/com/oracle/truffle/api/object

1 file changed

+61
-54
lines changed

‎truffle/src/com.oracle.truffle.api.object/src/com/oracle/truffle/api/object/DynamicObject.java‎

Lines changed: 61 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -604,7 +604,7 @@ public abstract static class PutNode extends Node {
604604
// @formatter:on
605605
@HostCompilerDirectives.InliningRoot
606606
public final void execute(DynamicObject receiver, Object key, Object value) {
607-
executeImpl(receiver, key, value, Flags.DEFAULT, 0);
607+
executeImpl(receiver, key, value, 0, Flags.DEFAULT);
608608
}
609609

610610
/**
@@ -618,7 +618,7 @@ public final void execute(DynamicObject receiver, Object key, Object value) {
618618
*/
619619
@HostCompilerDirectives.InliningRoot
620620
public final boolean executeIfPresent(DynamicObject receiver, Object key, Object value) {
621-
return executeImpl(receiver, key, value, Flags.IF_PRESENT, 0);
621+
return executeImpl(receiver, key, value, 0, Flags.IF_PRESENT);
622622
}
623623

624624
/**
@@ -632,7 +632,7 @@ public final boolean executeIfPresent(DynamicObject receiver, Object key, Object
632632
*/
633633
@HostCompilerDirectives.InliningRoot
634634
public final boolean executeIfAbsent(DynamicObject receiver, Object key, Object value) {
635-
return executeImpl(receiver, key, value, Flags.IF_ABSENT, 0);
635+
return executeImpl(receiver, key, value, 0, Flags.IF_ABSENT);
636636
}
637637

638638
/**
@@ -647,7 +647,7 @@ public final boolean executeIfAbsent(DynamicObject receiver, Object key, Object
647647
*/
648648
@HostCompilerDirectives.InliningRoot
649649
public final void executeWithFlags(DynamicObject receiver, Object key, Object value, int propertyFlags) {
650-
executeImpl(receiver, key, value, Flags.DEFAULT | Flags.UPDATE_FLAGS, propertyFlags);
650+
executeImpl(receiver, key, value, propertyFlags, Flags.DEFAULT | Flags.UPDATE_FLAGS);
651651
}
652652

653653
/**
@@ -656,7 +656,7 @@ public final void executeWithFlags(DynamicObject receiver, Object key, Object va
656656
*/
657657
@HostCompilerDirectives.InliningRoot
658658
public final boolean executeWithFlagsIfPresent(DynamicObject receiver, Object key, Object value, int propertyFlags) {
659-
return executeImpl(receiver, key, value, Flags.IF_PRESENT | Flags.UPDATE_FLAGS, propertyFlags);
659+
return executeImpl(receiver, key, value, propertyFlags, Flags.IF_PRESENT | Flags.UPDATE_FLAGS);
660660
}
661661

662662
/**
@@ -665,33 +665,30 @@ public final boolean executeWithFlagsIfPresent(DynamicObject receiver, Object ke
665665
*/
666666
@HostCompilerDirectives.InliningRoot
667667
public final boolean executeWithFlagsIfAbsent(DynamicObject receiver, Object key, Object value, int propertyFlags) {
668-
return executeImpl(receiver, key, value, Flags.IF_ABSENT | Flags.UPDATE_FLAGS, propertyFlags);
668+
return executeImpl(receiver, key, value, propertyFlags, Flags.IF_ABSENT | Flags.UPDATE_FLAGS);
669669
}
670670

671671
// private
672672

673-
abstract boolean executeImpl(DynamicObject receiver, Object key, Object value, int mode, int propertyFlags);
673+
abstract boolean executeImpl(DynamicObject receiver, Object key, Object value, int propertyFlags, int mode);
674674

675675
@SuppressWarnings("unused")
676676
@Specialization(guards = {
677677
"guard",
678678
"key == cachedKey",
679-
"mode == cachedMode",
680-
"propertyFlags == cachedPropertyFlags",
679+
"propertyFlagsEqual(propertyFlags, mode, oldShape, newShape, oldProperty, newProperty)",
681680
"newLocation == null || newLocation.canStoreValue(value)",
682-
}, assumptions = "newShapeValidAssumption", limit = "SHAPE_CACHE_LIMIT")
683-
static boolean doCached(DynamicObject receiver, Object key, Object value, int mode, int propertyFlags,
681+
}, assumptions = "oldShapeValidAssumption", limit = "SHAPE_CACHE_LIMIT")
682+
static boolean doCached(DynamicObject receiver, Object key, Object value, int propertyFlags, int mode,
684683
@Bind("receiver.getShape()") Shape shape,
685684
@Cached("shape") Shape oldShape,
686685
@Bind("shape == oldShape") boolean guard,
687686
@Cached("key") Object cachedKey,
688-
@Cached("mode") int cachedMode,
689-
@Cached("propertyFlags") int cachedPropertyFlags,
690687
@Cached("oldShape.getProperty(key)") Property oldProperty,
691-
@Cached("getNewShapeAndCheckOldShapeStillValid(key, value, cachedPropertyFlags, cachedMode, oldProperty, oldShape)") Shape newShape,
692-
@Cached("getNewLocation(oldShape, newShape, key, oldProperty)") Location newLocation,
693-
@Cached("newShape.getValidAbstractAssumption()") AbstractAssumption newShapeValidAssumption) {
694-
// We use mode instead of cachedMode to fold it during host inlining
688+
@Cached("getNewShape(key, value, propertyFlags, mode, oldProperty, oldShape)") Shape newShape,
689+
@Cached("getNewProperty(oldShape, newShape, key, oldProperty)") Property newProperty,
690+
@Cached("getLocation(newProperty)") Location newLocation,
691+
@Cached("oldShape.getValidAbstractAssumption()") AbstractAssumption oldShapeValidAssumption) {
695692
CompilerAsserts.partialEvaluationConstant(mode);
696693
if ((mode & Flags.IF_ABSENT) != 0 && oldProperty != null) {
697694
return false;
@@ -704,6 +701,7 @@ static boolean doCached(DynamicObject receiver, Object key, Object value, int mo
704701

705702
if (newShape != oldShape) {
706703
DynamicObjectSupport.setShapeWithStoreFence(receiver, newShape);
704+
maybeUpdateShape(receiver, newShape);
707705
}
708706
return true;
709707
}
@@ -715,13 +713,13 @@ static boolean doCached(DynamicObject receiver, Object key, Object value, int mo
715713
* still create new doCached instances, which is important once we see objects with the new
716714
* non-obsolete shape.
717715
*/
718-
@Specialization(guards = "!receiver.getShape().isValid()")
719-
static boolean doInvalid(DynamicObject receiver, Object key, Object value, int mode, int propertyFlags) {
716+
@Specialization(guards = "!receiver.getShape().isValid()", excludeForUncached = true)
717+
static boolean doInvalid(DynamicObject receiver, Object key, Object value, int propertyFlags, int mode) {
720718
return ObsolescenceStrategy.putGeneric(receiver, key, value, propertyFlags, mode);
721719
}
722720

723721
@Specialization(replaces = {"doCached", "doInvalid"})
724-
static boolean doGeneric(DynamicObject receiver, Object key, Object value, int mode, int propertyFlags) {
722+
static boolean doGeneric(DynamicObject receiver, Object key, Object value, int propertyFlags, int mode) {
725723
return ObsolescenceStrategy.putGeneric(receiver, key, value, propertyFlags, mode);
726724
}
727725

@@ -772,23 +770,23 @@ public abstract static class PutConstantNode extends Node {
772770
*/
773771
@HostCompilerDirectives.InliningRoot
774772
public final void execute(DynamicObject receiver, Object key, Object value) {
775-
executeImpl(receiver, key, value, Flags.DEFAULT | Flags.CONST, 0);
773+
executeImpl(receiver, key, value, 0, Flags.DEFAULT | Flags.CONST);
776774
}
777775

778776
/**
779777
* Like {@link #execute(DynamicObject, Object, Object)} but only if the property is present.
780778
*/
781779
@HostCompilerDirectives.InliningRoot
782780
public final boolean executeIfPresent(DynamicObject receiver, Object key, Object value) {
783-
return executeImpl(receiver, key, value, Flags.IF_PRESENT | Flags.CONST, 0);
781+
return executeImpl(receiver, key, value, 0, Flags.IF_PRESENT | Flags.CONST);
784782
}
785783

786784
/**
787785
* Like {@link #execute(DynamicObject, Object, Object)} but only if the property is absent.
788786
*/
789787
@HostCompilerDirectives.InliningRoot
790788
public final boolean executeIfAbsent(DynamicObject receiver, Object key, Object value) {
791-
return executeImpl(receiver, key, value, Flags.IF_ABSENT | Flags.CONST, 0);
789+
return executeImpl(receiver, key, value, 0, Flags.IF_ABSENT | Flags.CONST);
792790
}
793791

794792
// @formatter:off
@@ -831,7 +829,7 @@ public final boolean executeIfAbsent(DynamicObject receiver, Object key, Object
831829
// @formatter:on
832830
@HostCompilerDirectives.InliningRoot
833831
public final void executeWithFlags(DynamicObject receiver, Object key, Object value, int propertyFlags) {
834-
executeImpl(receiver, key, value, Flags.DEFAULT | Flags.CONST | Flags.UPDATE_FLAGS, propertyFlags);
832+
executeImpl(receiver, key, value, propertyFlags, Flags.DEFAULT | Flags.CONST | Flags.UPDATE_FLAGS);
835833
}
836834

837835
/**
@@ -840,7 +838,7 @@ public final void executeWithFlags(DynamicObject receiver, Object key, Object va
840838
*/
841839
@HostCompilerDirectives.InliningRoot
842840
public final boolean executeWithFlagsIfPresent(DynamicObject receiver, Object key, Object value, int propertyFlags) {
843-
return executeImpl(receiver, key, value, Flags.IF_PRESENT | Flags.CONST | Flags.UPDATE_FLAGS, propertyFlags);
841+
return executeImpl(receiver, key, value, propertyFlags, Flags.IF_PRESENT | Flags.CONST | Flags.UPDATE_FLAGS);
844842
}
845843

846844
/**
@@ -849,33 +847,29 @@ public final boolean executeWithFlagsIfPresent(DynamicObject receiver, Object ke
849847
*/
850848
@HostCompilerDirectives.InliningRoot
851849
public final boolean executeWithFlagsIfAbsent(DynamicObject receiver, Object key, Object value, int propertyFlags) {
852-
return executeImpl(receiver, key, value, Flags.IF_ABSENT | Flags.CONST | Flags.UPDATE_FLAGS, propertyFlags);
850+
return executeImpl(receiver, key, value, propertyFlags, Flags.IF_ABSENT | Flags.CONST | Flags.UPDATE_FLAGS);
853851
}
854852

855853
// private
856854

857-
abstract boolean executeImpl(DynamicObject receiver, Object key, Object value, int mode, int propertyFlags);
855+
abstract boolean executeImpl(DynamicObject receiver, Object key, Object value, int propertyFlags, int mode);
858856

859857
@SuppressWarnings("unused")
860858
@Specialization(guards = {
861859
"guard",
862860
"key == cachedKey",
863-
"mode == cachedMode",
864-
"propertyFlags == cachedPropertyFlags",
865-
"newLocation == null || newLocation.canStoreConstant(value)",
866-
}, assumptions = "newShapeValidAssumption", limit = "SHAPE_CACHE_LIMIT")
867-
static boolean doCached(DynamicObject receiver, Object key, Object value, int mode, int propertyFlags,
861+
"propertyFlagsEqual(propertyFlags, mode, oldShape, newShape, oldProperty, newProperty)",
862+
"newProperty == null || newProperty.getLocation().canStoreConstant(value)",
863+
}, assumptions = "oldShapeValidAssumption", limit = "SHAPE_CACHE_LIMIT")
864+
static boolean doCached(DynamicObject receiver, Object key, Object value, int propertyFlags, int mode,
868865
@Bind("receiver.getShape()") Shape shape,
869866
@Cached("shape") Shape oldShape,
870867
@Bind("shape == oldShape") boolean guard,
871868
@Cached("key") Object cachedKey,
872-
@Cached("mode") int cachedMode,
873-
@Cached("propertyFlags") int cachedPropertyFlags,
874869
@Cached("oldShape.getProperty(key)") Property oldProperty,
875-
@Cached("getNewShapeAndCheckOldShapeStillValid(key, value, cachedPropertyFlags, cachedMode, oldProperty, oldShape)") Shape newShape,
876-
@Cached("getNewLocation(oldShape, newShape, key, oldProperty)") Location newLocation,
877-
@Cached("newShape.getValidAbstractAssumption()") AbstractAssumption newShapeValidAssumption) {
878-
// We use mode instead of cachedMode to fold it during host inlining
870+
@Cached("getNewShape(key, value, propertyFlags, mode, oldProperty, oldShape)") Shape newShape,
871+
@Cached("getNewProperty(oldShape, newShape, key, oldProperty)") Property newProperty,
872+
@Cached("oldShape.getValidAbstractAssumption()") AbstractAssumption oldShapeValidAssumption) {
879873
CompilerAsserts.partialEvaluationConstant(mode);
880874
if ((mode & Flags.IF_ABSENT) != 0 && oldProperty != null) {
881875
return false;
@@ -886,6 +880,7 @@ static boolean doCached(DynamicObject receiver, Object key, Object value, int mo
886880

887881
if (newShape != oldShape) {
888882
DynamicObjectSupport.setShapeWithStoreFence(receiver, newShape);
883+
maybeUpdateShape(receiver, newShape);
889884
}
890885
return true;
891886
}
@@ -897,13 +892,13 @@ static boolean doCached(DynamicObject receiver, Object key, Object value, int mo
897892
* still create new doCached instances, which is important once we see objects with the new
898893
* non-obsolete shape.
899894
*/
900-
@Specialization(guards = "!receiver.getShape().isValid()")
901-
static boolean doInvalid(DynamicObject receiver, Object key, Object value, int mode, int propertyFlags) {
895+
@Specialization(guards = "!receiver.getShape().isValid()", excludeForUncached = true)
896+
static boolean doInvalid(DynamicObject receiver, Object key, Object value, int propertyFlags, int mode) {
902897
return ObsolescenceStrategy.putGeneric(receiver, key, value, propertyFlags, mode);
903898
}
904899

905900
@Specialization(replaces = {"doCached", "doInvalid"})
906-
static boolean doGeneric(DynamicObject receiver, Object key, Object value, int mode, int propertyFlags) {
901+
static boolean doGeneric(DynamicObject receiver, Object key, Object value, int propertyFlags, int mode) {
907902
return ObsolescenceStrategy.putGeneric(receiver, key, value, propertyFlags, mode);
908903
}
909904

@@ -924,6 +919,24 @@ public static PutConstantNode getUncached() {
924919
}
925920
}
926921

922+
/**
923+
* {@return true if the cache entry can be used with the passed property flags and mode}
924+
*
925+
* <ol>
926+
* <li>ignore flags => new flags must equal old flags, if any, else passed flags</li>
927+
* <li>update flags => new flags must equal passed flags</li>
928+
* </ol>
929+
*/
930+
static boolean propertyFlagsEqual(int propertyFlags, int mode, Shape oldShape, Shape newShape, Property oldProperty, Property newProperty) {
931+
if (newProperty == null) {
932+
assert oldProperty == null;
933+
return (mode & Flags.IF_PRESENT) != 0;
934+
}
935+
return (mode & Flags.UPDATE_FLAGS) == 0
936+
? oldShape == newShape || (oldProperty == null ? propertyFlags : oldProperty.getFlags()) == newProperty.getFlags()
937+
: propertyFlags == newProperty.getFlags();
938+
}
939+
927940
static Shape getNewShape(Object cachedKey, Object value, int newPropertyFlags, int mode, Property existingProperty, Shape oldShape) {
928941
if (existingProperty == null) {
929942
if ((mode & Flags.IF_PRESENT) != 0) {
@@ -947,24 +960,18 @@ static Shape getNewShape(Object cachedKey, Object value, int newPropertyFlags, i
947960
}
948961
}
949962

950-
// defineProperty() might obsolete the oldShape and we don't handle invalid shape -> valid
951-
// shape transitions on the fast path (in doCached)
952-
static Shape getNewShapeAndCheckOldShapeStillValid(Object cachedKey, Object value, int newPropertyFlags, int putFlags, Property existingProperty, Shape oldShape) {
953-
Shape newShape = getNewShape(cachedKey, value, newPropertyFlags, putFlags, existingProperty, oldShape);
954-
if (!oldShape.isValid()) {
955-
return oldShape; // return an invalid shape to not use this specialization
956-
}
957-
return newShape;
958-
}
959-
960-
static Location getNewLocation(Shape oldShape, Shape newShape, Object cachedKey, Property oldProperty) {
963+
static Property getNewProperty(Shape oldShape, Shape newShape, Object cachedKey, Property oldProperty) {
961964
if (newShape == oldShape) {
962-
return oldProperty == null ? null : oldProperty.getLocation();
965+
return oldProperty;
963966
} else {
964-
return newShape.getLocation(cachedKey);
967+
return newShape.getProperty(cachedKey);
965968
}
966969
}
967970

971+
static Location getLocation(Property property) {
972+
return property == null ? null : property.getLocation();
973+
}
974+
968975
/**
969976
* Copies all properties of a DynamicObject to another, preserving property flags. Does not copy
970977
* hidden properties.

0 commit comments

Comments
 (0)