Skip to content

Commit 6f0e413

Browse files
eregoneregon
committed
[GR-16458] Fix ObjectSpace.trace_object_allocations.
PullRequest: truffleruby/894
2 parents 2465104 + 72a48c8 commit 6f0e413

File tree

7 files changed

+56
-60
lines changed

7 files changed

+56
-60
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ Bug fixes:
88
* Fixed `Process.kill(signal, Process.pid)` when the signal is trapped as `:IGNORE` (#1702).
99
* Fixed `Addrinfo.new(String)` to reliably find the address family (#1702).
1010
* Fixed argument checks in `BasicSocket#setsockopt` (#1460).
11+
* Fixed `ObjectSpace.trace_object_allocations` (#1456).
1112

1213
Compatibility:
1314

‎lib/truffle/objspace.rb

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,6 @@ def reachable_objects_from_root
184184

185185
def trace_object_allocations
186186
trace_object_allocations_start
187-
188187
begin
189188
yield
190189
ensure
@@ -230,7 +229,11 @@ def allocation_generation(object)
230229
def allocation_method_id(object)
231230
allocation = ALLOCATIONS[object]
232231
return nil if allocation.nil?
233-
allocation.method_id
232+
233+
method_id = allocation.method_id
234+
# The allocator function is hidden in MRI
235+
method_id = :new if method_id == :__allocate__
236+
method_id
234237
end
235238
module_function :allocation_method_id
236239

‎src/main/java/org/truffleruby/core/array/ArrayLiteralNode.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
import org.truffleruby.core.CoreLibrary;
1818
import org.truffleruby.language.RubyNode;
1919
import org.truffleruby.language.objects.AllocateObjectNode;
20-
import org.truffleruby.language.objects.AllocateObjectNodeGen;
2120

2221
public abstract class ArrayLiteralNode extends RubyNode {
2322

@@ -54,7 +53,7 @@ protected DynamicObject makeGeneric(VirtualFrame frame, Object[] alreadyExecuted
5453
protected DynamicObject createArray(Object store, int size) {
5554
if (allocateObjectNode == null) {
5655
CompilerDirectives.transferToInterpreterAndInvalidate();
57-
allocateObjectNode = insert(AllocateObjectNodeGen.create(false));
56+
allocateObjectNode = insert(AllocateObjectNode.create());
5857
}
5958
return allocateObjectNode.allocate(coreLibrary().getArrayClass(), store, size);
6059
}

‎src/main/java/org/truffleruby/core/objectspace/ObjectSpaceManager.java

Lines changed: 15 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -40,14 +40,15 @@
4040

4141
import com.oracle.truffle.api.Assumption;
4242
import com.oracle.truffle.api.CompilerDirectives;
43-
import com.oracle.truffle.api.object.DynamicObject;
43+
import com.oracle.truffle.api.CompilerDirectives.CompilationFinal;
4444
import com.oracle.truffle.api.utilities.CyclicAssumption;
4545

4646
import org.truffleruby.RubyContext;
4747
import org.truffleruby.cext.ValueWrapperManager;
4848

4949
import java.lang.management.GarbageCollectorMXBean;
5050
import java.lang.management.ManagementFactory;
51+
import java.util.concurrent.atomic.AtomicInteger;
5152
import java.util.concurrent.atomic.AtomicLong;
5253

5354
/**
@@ -59,9 +60,9 @@ public class ObjectSpaceManager {
5960
private final RubyContext context;
6061

6162
private final CyclicAssumption tracingAssumption = new CyclicAssumption("objspace-tracing");
62-
@CompilerDirectives.CompilationFinal private boolean isTracing = false;
63-
private int tracingAssumptionActivations = 0;
64-
private boolean tracingPaused = false;
63+
@CompilationFinal private boolean isTracing = false;
64+
private final AtomicInteger tracingAssumptionActivations = new AtomicInteger(0);
65+
private final ThreadLocal<Boolean> tracingPaused = ThreadLocal.withInitial(() -> false);
6566

6667
private final AtomicLong nextObjectID = new AtomicLong(ValueWrapperManager.TAG_MASK + 1);
6768

@@ -70,37 +71,19 @@ public ObjectSpaceManager(RubyContext context) {
7071
}
7172

7273
public void traceAllocationsStart() {
73-
tracingAssumptionActivations++;
74-
75-
if (tracingAssumptionActivations == 1) {
74+
if (tracingAssumptionActivations.incrementAndGet() == 1) {
7675
isTracing = true;
7776
tracingAssumption.invalidate();
7877
}
7978
}
8079

8180
public void traceAllocationsStop() {
82-
tracingAssumptionActivations--;
83-
84-
if (tracingAssumptionActivations == 0) {
81+
if (tracingAssumptionActivations.decrementAndGet() == 0) {
8582
isTracing = false;
8683
tracingAssumption.invalidate();
8784
}
8885
}
8986

90-
public void traceAllocation(DynamicObject object, DynamicObject classPath, DynamicObject methodId, DynamicObject sourcefile, int sourceline) {
91-
if (tracingPaused) {
92-
return;
93-
}
94-
95-
tracingPaused = true;
96-
97-
try {
98-
context.send(context.getCoreLibrary().getObjectSpaceModule(), "trace_allocation", object, classPath, methodId, sourcefile, sourceline, getCollectionCount());
99-
} finally {
100-
tracingPaused = false;
101-
}
102-
}
103-
10487
public Assumption getTracingAssumption() {
10588
return tracingAssumption.getAssumption();
10689
}
@@ -109,6 +92,14 @@ public boolean isTracing() {
10992
return isTracing;
11093
}
11194

95+
public boolean isTracingPaused() {
96+
return tracingPaused.get();
97+
}
98+
99+
public void setTracingPaused(boolean tracingPaused) {
100+
this.tracingPaused.set(tracingPaused);
101+
}
102+
112103
public long getNextObjectID() {
113104
final long id = nextObjectID.getAndAdd(ValueWrapperManager.TAG_MASK + 1);
114105

‎src/main/java/org/truffleruby/language/literal/StringLiteralNode.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,15 @@
2020

2121
import com.oracle.truffle.api.frame.VirtualFrame;
2222
import com.oracle.truffle.api.object.DynamicObject;
23+
import org.truffleruby.Layouts;
2324
import org.truffleruby.core.rope.Rope;
24-
import org.truffleruby.core.string.StringOperations;
2525
import org.truffleruby.language.RubyNode;
26+
import org.truffleruby.language.objects.AllocateObjectNode;
2627

2728
public class StringLiteralNode extends RubyNode {
2829

30+
@Child AllocateObjectNode allocateNode = AllocateObjectNode.create();
31+
2932
private final Rope rope;
3033

3134
public StringLiteralNode(Rope rope) {
@@ -35,7 +38,7 @@ public StringLiteralNode(Rope rope) {
3538

3639
@Override
3740
public DynamicObject execute(VirtualFrame frame) {
38-
return StringOperations.createString(getContext(), rope);
41+
return allocateNode.allocate(coreLibrary().getStringClass(), Layouts.STRING.build(false, false, rope));
3942
}
4043

4144
}

‎src/main/java/org/truffleruby/language/objects/AllocateObjectNode.java

Lines changed: 29 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@
2323
import com.oracle.truffle.api.source.SourceSection;
2424
import org.jcodings.specific.UTF8Encoding;
2525
import org.truffleruby.Layouts;
26-
import org.truffleruby.core.rope.CodeRange;
27-
import org.truffleruby.core.string.StringNodes;
26+
import org.truffleruby.core.objectspace.ObjectSpaceManager;
27+
import org.truffleruby.core.string.StringOperations;
2828
import org.truffleruby.language.RubyBaseNode;
2929
import org.truffleruby.language.arguments.RubyArguments;
3030
import org.truffleruby.language.control.RaiseException;
@@ -33,13 +33,7 @@
3333
public abstract class AllocateObjectNode extends RubyBaseNode {
3434

3535
public static AllocateObjectNode create() {
36-
return AllocateObjectNodeGen.create(true);
37-
}
38-
39-
private final boolean useCallerFrameForTracing;
40-
41-
public AllocateObjectNode(boolean useCallerFrameForTracing) {
42-
this.useCallerFrameForTracing = useCallerFrameForTracing;
36+
return AllocateObjectNodeGen.create();
4337
}
4438

4539
public DynamicObject allocate(DynamicObject classToAllocate, Object... values) {
@@ -81,42 +75,49 @@ public DynamicObject allocateUncached(DynamicObject classToAllocate, Object[] va
8175
@CompilerDirectives.TruffleBoundary
8276
@Specialization(guards = {"!isSingleton(classToAllocate)", "isTracing()"},
8377
assumptions = "getTracingAssumption()")
84-
public DynamicObject allocateTracing(DynamicObject classToAllocate, Object[] values,
85-
@Cached("create()") StringNodes.MakeStringNode makeStringNode) {
78+
public DynamicObject allocateTracing(DynamicObject classToAllocate, Object[] values) {
8679
final DynamicObject object = allocate(getInstanceFactory(classToAllocate), values);
8780

88-
final FrameInstance allocatingFrameInstance;
89-
final SourceSection allocatingSourceSection;
90-
91-
if (useCallerFrameForTracing) {
92-
allocatingFrameInstance = getContext().getCallStack().getCallerFrameIgnoringSend();
93-
allocatingSourceSection = getContext().getCallStack().getTopMostUserSourceSection();
94-
} else {
95-
allocatingFrameInstance = Truffle.getRuntime().getCurrentFrame();
96-
allocatingSourceSection = getEncapsulatingSourceSection();
81+
final ObjectSpaceManager objectSpaceManager = getContext().getObjectSpaceManager();
82+
if (!objectSpaceManager.isTracingPaused()) {
83+
objectSpaceManager.setTracingPaused(true);
84+
try {
85+
callTraceAllocation(object);
86+
} finally {
87+
objectSpaceManager.setTracingPaused(false);
88+
}
9789
}
9890

91+
return object;
92+
}
93+
94+
@CompilerDirectives.TruffleBoundary
95+
private void callTraceAllocation(DynamicObject object) {
96+
final SourceSection allocatingSourceSection = getContext().getCallStack().getTopMostUserSourceSection(getEncapsulatingSourceSection());
97+
98+
final FrameInstance allocatingFrameInstance = Truffle.getRuntime().getCurrentFrame();
9999
final Frame allocatingFrame = allocatingFrameInstance.getFrame(FrameInstance.FrameAccess.READ_ONLY);
100100

101101
final Object allocatingSelf = RubyArguments.getSelf(allocatingFrame);
102102
final String allocatingMethod = RubyArguments.getMethod(allocatingFrame).getName();
103103

104-
getContext().getObjectSpaceManager().traceAllocation(
104+
getContext().send(coreLibrary().getObjectSpaceModule(), "trace_allocation",
105105
object,
106-
string(makeStringNode, Layouts.CLASS.getFields(coreLibrary().getLogicalClass(allocatingSelf)).getName()),
106+
string(Layouts.CLASS.getFields(coreLibrary().getLogicalClass(allocatingSelf)).getName()),
107107
getSymbol(allocatingMethod),
108-
string(makeStringNode, getContext().getPath(allocatingSourceSection.getSource())),
109-
allocatingSourceSection.getStartLine());
110-
111-
return object;
108+
string(getContext().getPath(allocatingSourceSection.getSource())),
109+
allocatingSourceSection.getStartLine(),
110+
ObjectSpaceManager.getCollectionCount());
112111
}
113112

114113
protected DynamicObjectFactory getInstanceFactory(DynamicObject classToAllocate) {
115114
return Layouts.CLASS.getInstanceFactory(classToAllocate);
116115
}
117116

118-
private DynamicObject string(StringNodes.MakeStringNode makeStringNode, String value) {
119-
return makeStringNode.executeMake(value, UTF8Encoding.INSTANCE, CodeRange.CR_UNKNOWN);
117+
private DynamicObject string(String value) {
118+
// No point to use MakeStringNode (which uses AllocateObjectNode) here, as we should not trace the allocation
119+
// of Strings used for tracing allocations.
120+
return StringOperations.createString(getContext(), StringOperations.encodeRope(value, UTF8Encoding.INSTANCE));
120121
}
121122

122123
@Specialization(guards = "isSingleton(classToAllocate)")

‎test/mri/excludes/TestObjSpace.rb

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,6 @@
1818
exclude :test_memsize_of_iseq, "needs investigation"
1919
exclude :test_memsize_of_root_shared_string, "needs investigation"
2020
exclude :test_reachable_objects_size, "needs investigation"
21-
exclude :test_trace_object_allocations, "needs investigation"
22-
exclude :test_trace_object_allocations_start_stop_clear, "needs investigation"
2321
exclude :test_count_objects_size_with_wrong_type, "needs investigation"
2422
exclude :test_dump_addresses_match_dump_all_addresses, "needs investigation"
2523
exclude :test_dump_all_full, "needs investigation"

0 commit comments

Comments
 (0)