-
-
Notifications
You must be signed in to change notification settings - Fork 925
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[Truffle] Removed boundary for
string_substring
primitive.
This primitive is used by String#[] amongst others.
- 9.4.12.0
- 9.4.11.0
- 9.4.10.0
- 9.4.9.0
- 9.4.8.0
- 9.4.7.0
- 9.4.6.0
- 9.4.5.0
- 9.4.4.0
- 9.4.3.0
- 9.4.2.0
- 9.4.1.0
- 9.4.0.0
- 9.3.15.0
- 9.3.14.0
- 9.3.13.0
- 9.3.12.0
- 9.3.11.0
- 9.3.10.0
- 9.3.9.0
- 9.3.8.0
- 9.3.7.0
- 9.3.6.0
- 9.3.5.0
- 9.3.4.0
- 9.3.3.0
- 9.3.2.0
- 9.3.1.0
- 9.3.0.0
- 9.2.21.0
- 9.2.20.1
- 9.2.20.0
- 9.2.19.0
- 9.2.18.0
- 9.2.17.0
- 9.2.16.0
- 9.2.15.0
- 9.2.14.0
- 9.2.13.0
- 9.2.12.0
- 9.2.11.1
- 9.2.11.0
- 9.2.10.0
- 9.2.9.0
- 9.2.8.0
- 9.2.7.0
- 9.2.6.0
- 9.2.5.0
- 9.2.4.1
- 9.2.4.0
- 9.2.3.0
- 9.2.2.0
- 9.2.1.0
- 9.2.0.0
- 9.1.17.0
- 9.1.16.0
- 9.1.15.0
- 9.1.14.0
- 9.1.13.0
- 9.1.12.0
- 9.1.11.0
- 9.1.10.0
- 9.1.9.0
- 9.1.8.0
- 9.1.7.0
- 9.1.6.0
- 9.1.5.0
- 9.1.4.0
- 9.1.3.0
- 9.1.2.0
- 9.1.1.0
- 9.1.0.0
- 9.0.5.0
- 9.0.4.0
- 9.0.3.0
Showing
1 changed file
with
18 additions
and
6 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1188,20 +1188,23 @@ public DynamicObject stringByteAppend(DynamicObject string, DynamicObject other) | |
@ImportStatic(StringGuards.class) | ||
public static abstract class StringSubstringPrimitiveNode extends RubiniusPrimitiveNode { | ||
|
||
@Child private AllocateObjectNode allocateNode; | ||
@Child private TaintResultNode taintResultNode; | ||
private final ConditionProfile negativeLengthProfile = ConditionProfile.createBinaryProfile(); | ||
private final ConditionProfile tooLargeBeginProfile = ConditionProfile.createBinaryProfile(); | ||
private final ConditionProfile negativeBeginProfile = ConditionProfile.createBinaryProfile(); | ||
This comment has been minimized.
Sorry, something went wrong.
This comment has been minimized.
Sorry, something went wrong.
chrisseaton
Contributor
|
||
|
||
public StringSubstringPrimitiveNode(RubyContext context, SourceSection sourceSection) { | ||
super(context, sourceSection); | ||
} | ||
|
||
public abstract Object execute(VirtualFrame frame, DynamicObject string, int beg, int len); | ||
|
||
@TruffleBoundary | ||
@Specialization(guards = "isSingleByteOptimizable(string)") | ||
public Object stringSubstringSingleByteOptimizable(DynamicObject string, int beg, int len) { | ||
// Taken from org.jruby.RubyString#substr19. | ||
|
||
if (len < 0) { | ||
if (negativeLengthProfile.profile(len < 0)) { | ||
return nil(); | ||
} | ||
|
||
|
@@ -1210,14 +1213,14 @@ public Object stringSubstringSingleByteOptimizable(DynamicObject string, int beg | |
len = 0; | ||
} | ||
|
||
if (beg > length) { | ||
if (tooLargeBeginProfile.profile(beg > length)) { | ||
return nil(); | ||
} | ||
|
||
if (beg < 0) { | ||
beg += length; | ||
|
||
if (beg < 0) { | ||
if (negativeBeginProfile.profile(beg < 0)) { | ||
return nil(); | ||
} | ||
} | ||
|
@@ -1310,13 +1313,22 @@ public Object stringSubstring(DynamicObject string, int beg, int len) { | |
private DynamicObject makeSubstring(DynamicObject string, int beg, int len) { | ||
assert RubyGuards.isRubyString(string); | ||
|
||
if (allocateNode == null) { | ||
CompilerDirectives.transferToInterpreter(); | ||
allocateNode = insert(AllocateObjectNodeGen.create(getContext(), getSourceSection(), null, null)); | ||
} | ||
|
||
if (taintResultNode == null) { | ||
CompilerDirectives.transferToInterpreter(); | ||
taintResultNode = insert(new TaintResultNode(getContext(), getSourceSection())); | ||
} | ||
|
||
final DynamicObject ret = Layouts.STRING.createString(Layouts.CLASS.getInstanceFactory(Layouts.BASIC_OBJECT.getLogicalClass(string)), new ByteList(StringOperations.getByteList(string), beg, len), StringSupport.CR_UNKNOWN, null); | ||
StringOperations.getByteList(ret).setEncoding(StringOperations.getByteList(string).getEncoding()); | ||
final DynamicObject ret = allocateNode.allocate( | ||
Layouts.BASIC_OBJECT.getLogicalClass(string), | ||
new ByteList(StringOperations.getByteList(string).unsafeBytes(), beg, len), | ||
StringSupport.CR_UNKNOWN, | ||
null); | ||
|
||
taintResultNode.maybeTaint(string, ret); | ||
|
||
return ret; | ||
|
Profiles should in general not be defined in the base node itself, as there only one instance of that one for N specializations. In this case it's fine because they are only used in one specialization but if they were in multiple specializations it would cause profile pollution between specializations.
A way to have per-specialization profiles as it is supposed to be is with
@Cached
, as in AppendOneNode for example. It also avoid allocating the profile if not used.@chrisseaton Am I correct? Should we try to unify the style for profiles?