Skip to content

Commit 001f6a5

Browse files
DSouzaMDSouzaM
committed
[GR-70561] [GR-63990] [GR-70663] [GR-70704] Miscellaneous Bytecode DSL and Truffle fixes.
PullRequest: graal/22336
2 parents 5232606 + d5da212 commit 001f6a5

File tree

10 files changed

+168
-106
lines changed

10 files changed

+168
-106
lines changed

‎truffle/mx.truffle/mx_polybench/command.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,7 @@ def _validate_jdk(run_spec: "PolybenchRunSpecification") -> mx.JDKConfig:
281281
'(or a GraalVM built from source, e.g., with "mx -p /vm --env ce build").'
282282
)
283283
if not mx_sdk.GraalVMJDKConfig.is_graalvm(jdk.home):
284-
if run_spec.is_native:
284+
if run_spec.is_native():
285285
mx.abort(
286286
f"Polybench was invoked with a non-Graal JDK ({jdk.home}), but a native image run was requested. "
287287
f"Re-run using a Graal JDK. " + rerun_details

‎truffle/src/com.oracle.truffle.dsl.processor/src/com/oracle/truffle/dsl/processor/bytecode/generator/BytecodeRootNodeElement.java‎

Lines changed: 81 additions & 65 deletions
Large diffs are not rendered by default.

‎truffle/src/com.oracle.truffle.dsl.processor/src/com/oracle/truffle/dsl/processor/bytecode/model/BytecodeDSLModel.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,7 @@ public TypeMirror getProvidedRootBodyTag() {
271271
}
272272

273273
public boolean isBytecodeUpdatable() {
274-
return !getInstrumentations().isEmpty() || !getProvidedTags().isEmpty();
274+
return !getInstrumentations().isEmpty() || (enableTagInstrumentation && !getProvidedTags().isEmpty());
275275
}
276276

277277
public boolean hasYieldOperation() {
@@ -281,8 +281,9 @@ public boolean hasYieldOperation() {
281281
public InstructionModel getInvalidateInstruction(int length) {
282282
if (invalidateInstructions == null) {
283283
return null;
284+
} else if (length % 2 != 0) {
285+
throw new AssertionError("instructions must be short-aligned");
284286
}
285-
assert length % 2 == 0;
286287
return invalidateInstructions[(length - OPCODE_WIDTH) / 2];
287288
}
288289

‎truffle/src/com.oracle.truffle.dsl.processor/src/com/oracle/truffle/dsl/processor/bytecode/model/InstructionModel.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -639,11 +639,15 @@ public InstructionEncoding getInstructionEncoding() {
639639
public String getInternalName() {
640640
String operationName = switch (kind) {
641641
case CUSTOM -> {
642-
assert name.startsWith("c.");
642+
if (!name.startsWith("c.")) {
643+
throw new AssertionError("Unexpected custom operation name: " + name);
644+
}
643645
yield name.substring(2) + "_";
644646
}
645647
case CUSTOM_SHORT_CIRCUIT -> {
646-
assert name.startsWith("sc.");
648+
if (!name.startsWith("sc.")) {
649+
throw new AssertionError("Unexpected short-circuit custom operation name: " + name);
650+
}
647651
yield name.substring(3) + "_";
648652
}
649653
default -> name;

‎truffle/src/com.oracle.truffle.dsl.processor/src/com/oracle/truffle/dsl/processor/bytecode/model/OperationModel.java‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -274,16 +274,18 @@ public OperationModel setOperationBeginArgumentVarArgs(boolean varArgs) {
274274
}
275275

276276
public OperationModel setOperationBeginArguments(OperationArgument... operationBeginArguments) {
277-
if (this.operationBeginArguments != null) {
278-
assert this.operationBeginArguments.length == operationBeginArguments.length;
277+
if (this.operationBeginArguments != EMPTY_ARGUMENTS && this.operationBeginArguments.length != operationBeginArguments.length) {
278+
throw new AssertionError("Number of begin arguments for %s should not change (was %d, attempted to set to %d).".formatted(name, this.operationBeginArguments.length,
279+
operationBeginArguments.length));
279280
}
280281
this.operationBeginArguments = operationBeginArguments;
281282
return this;
282283
}
283284

284285
public OperationModel setOperationEndArguments(OperationArgument... operationEndArguments) {
285-
if (this.operationEndArguments != null) {
286-
assert this.operationEndArguments.length == operationEndArguments.length;
286+
if (this.operationEndArguments != EMPTY_ARGUMENTS && this.operationEndArguments.length != operationEndArguments.length) {
287+
throw new AssertionError(
288+
"Number of end arguments for %s should not change (was %d, attempted to set to %d).".formatted(name, this.operationEndArguments.length, operationEndArguments.length));
287289
}
288290
this.operationEndArguments = operationEndArguments;
289291
return this;

‎truffle/src/com.oracle.truffle.dsl.processor/src/com/oracle/truffle/dsl/processor/bytecode/parser/BytecodeDSLParser.java‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,9 @@ protected BytecodeDSLModels parse(Element element, List<AnnotationMirror> mirror
132132
models = parseGenerateBytecodeTestVariants(typeElement, generateBytecodeTestVariantsMirror);
133133
} else {
134134
AnnotationMirror generateBytecodeMirror = ElementUtils.findAnnotationMirror(element.getAnnotationMirrors(), types.GenerateBytecode);
135-
assert generateBytecodeMirror != null;
135+
if (generateBytecodeMirror == null) {
136+
throw new AssertionError("No @%s mirror found.".formatted(getSimpleName(types.GenerateBytecode)));
137+
}
136138
topLevelAnnotationMirror = generateBytecodeMirror;
137139

138140
models = List.of(createBytecodeDSLModel(typeElement, generateBytecodeMirror, "Gen", types.BytecodeBuilder));
@@ -369,11 +371,15 @@ private void parseBytecodeDSLModel(TypeElement typeElement, BytecodeDSLModel mod
369371
// Prioritize a constructor that takes a FrameDescriptor.Builder.
370372
if (constructorsByFDType.containsKey(TruffleTypes.FrameDescriptor_Builder_Name)) {
371373
List<ExecutableElement> ctors = constructorsByFDType.get(TruffleTypes.FrameDescriptor_Builder_Name);
372-
assert ctors.size() == 1;
374+
if (ctors.size() != 1) {
375+
throw new AssertionError();
376+
}
373377
model.fdBuilderConstructor = ctors.get(0);
374378
} else {
375379
List<ExecutableElement> ctors = constructorsByFDType.get(TruffleTypes.FrameDescriptor_Name);
376-
assert ctors.size() == 1;
380+
if (ctors.size() != 1) {
381+
throw new AssertionError();
382+
}
377383
model.fdConstructor = ctors.get(0);
378384
}
379385

@@ -822,7 +828,9 @@ private void resolveBoxingElimination(BytecodeDSLModel model, List<QuickenDecisi
822828
List<ResolvedQuickenDecision> decisions = entry.getValue();
823829

824830
for (ResolvedQuickenDecision quickening : decisions) {
825-
assert !includedSpecializations.isEmpty();
831+
if (includedSpecializations.isEmpty()) {
832+
throw new AssertionError();
833+
}
826834
NodeData node = quickening.operation().instruction.nodeData;
827835

828836
String name;

‎truffle/src/com.oracle.truffle.dsl.processor/src/com/oracle/truffle/dsl/processor/bytecode/parser/CustomOperationParser.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -603,7 +603,9 @@ private void validateEpilogExceptionalSignature(CustomOperationModel customOpera
603603
}
604604

605605
private OperationArgument[] createOperationConstantArguments(List<ConstantOperandModel> operands, List<String> operandNames) {
606-
assert operands.size() == operandNames.size();
606+
if (operands.size() != operandNames.size()) {
607+
throw new AssertionError("Operands and operand names have different sizes (%d vs. %d)".formatted(operands.size(), operandNames.size()));
608+
}
607609
OperationArgument[] arguments = new OperationArgument[operandNames.size()];
608610
for (int i = 0; i < operandNames.size(); i++) {
609611
ConstantOperandModel constantOperand = operands.get(i);
@@ -673,7 +675,9 @@ private InstructionModel getOrCreateBooleanConverterInstruction(TypeElement type
673675
}
674676

675677
List<ExecutableElement> specializations = findSpecializations(typeElement);
676-
assert specializations.size() != 0;
678+
if (specializations.isEmpty()) {
679+
throw new AssertionError("Boolean converter should have at least one specialization if it parsed without error.");
680+
}
677681

678682
boolean returnsBoolean = true;
679683
for (ExecutableElement spec : specializations) {

‎truffle/src/com.oracle.truffle.dsl.processor/src/com/oracle/truffle/dsl/processor/bytecode/parser/SpecializationSignatureParser.java‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -250,8 +250,11 @@ private boolean checkConstantOperandParam(VariableElement constantOperandParam,
250250
* inspected individually.
251251
*/
252252
public static Signature createPolymorphicSignature(List<SpecializationSignature> signatures, List<ExecutableElement> specializations, MessageContainer customOperation) {
253-
assert !signatures.isEmpty();
254-
assert signatures.size() == specializations.size();
253+
if (signatures.isEmpty()) {
254+
throw new IllegalArgumentException("Cannot create a polymorphic signature for an empty list of signatures.");
255+
} else if (signatures.size() != specializations.size()) {
256+
throw new IllegalArgumentException("Number of signatures (%d) should match number of specializations (%d).".formatted(signatures.size(), specializations.size()));
257+
}
255258
Signature polymorphicSignature = signatures.get(0).signature();
256259
for (int i = 1; i < signatures.size(); i++) {
257260
polymorphicSignature = mergeSignatures(signatures.get(i).signature(), polymorphicSignature, specializations.get(i), customOperation);
@@ -276,8 +279,11 @@ private static Signature mergeSignatures(Signature a, Signature b, Element el, M
276279
}
277280
return null;
278281
}
279-
assert a.constantOperandsBeforeCount == b.constantOperandsBeforeCount;
280-
assert a.constantOperandsAfterCount == b.constantOperandsAfterCount;
282+
if (a.constantOperandsBeforeCount != b.constantOperandsBeforeCount) {
283+
throw new AssertionError("Constant operand before count differs between merged signatures (%d and %d).".formatted(a.constantOperandsBeforeCount, b.constantOperandsBeforeCount));
284+
} else if (a.constantOperandsAfterCount != b.constantOperandsAfterCount) {
285+
throw new AssertionError("Constant operand counts should match between merged signatures (%d and %d).".formatted(a.constantOperandsAfterCount, b.constantOperandsAfterCount));
286+
}
281287
if (a.dynamicOperandCount != b.dynamicOperandCount) {
282288
if (errorTarget != null) {
283289
errorTarget.addError(el, "Error calculating operation signature: all specializations must have the same number of operands.");

‎truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotStackFramesRetriever.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
*/
4141
package com.oracle.truffle.polyglot;
4242

43+
import java.util.ArrayList;
4344
import java.util.List;
4445
import java.util.concurrent.CompletableFuture;
4546
import java.util.concurrent.ExecutionException;
@@ -65,13 +66,17 @@ static void populateHeapRoots(PolyglotContextImpl context, List<Object> heapRoot
6566
future = context.threadLocalActions.submit(null, PolyglotEngineImpl.ENGINE_ID, new ThreadLocalAction(false, false) {
6667
@Override
6768
protected void perform(Access access) {
69+
List<Object> threadHeapRoots = new ArrayList<>();
6870
Truffle.getRuntime().iterateFrames(new FrameInstanceVisitor<>() {
6971
@Override
7072
public Object visitFrame(FrameInstance frameInstance) {
71-
populateHeapRootsFromFrame(context, frameInstance, heapRoots);
73+
populateHeapRootsFromFrame(context, frameInstance, threadHeapRoots);
7274
return null;
7375
}
7476
});
77+
synchronized (heapRoots) {
78+
heapRoots.addAll(threadHeapRoots);
79+
}
7580
}
7681
}, false);
7782
} else {

‎truffle/src/org.graalvm.polybench.instruments/src/org/graalvm/polybench/instruments/MemoryUsageInstrument.java‎

Lines changed: 37 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,6 @@
3030
import java.util.LongSummaryStatistics;
3131
import java.util.Map;
3232
import java.util.Set;
33-
import java.util.Timer;
34-
import java.util.TimerTask;
3533
import java.util.concurrent.CancellationException;
3634
import java.util.concurrent.ConcurrentHashMap;
3735
import java.util.concurrent.Future;
@@ -46,6 +44,7 @@
4644
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
4745
import com.oracle.truffle.api.Option;
4846
import com.oracle.truffle.api.TruffleContext;
47+
import com.oracle.truffle.api.TruffleSafepoint;
4948
import com.oracle.truffle.api.instrumentation.ContextsListener;
5049
import com.oracle.truffle.api.instrumentation.ThreadsListener;
5150
import com.oracle.truffle.api.instrumentation.TruffleInstrument;
@@ -94,13 +93,6 @@ protected OptionDescriptors getOptionDescriptors() {
9493
return new MemoryUsageInstrumentOptionDescriptors();
9594
}
9695

97-
@TruffleBoundary
98-
public long getContextHeapSize() {
99-
TruffleContext context = currentEnv.getEnteredContext();
100-
AtomicBoolean b = new AtomicBoolean();
101-
return currentEnv.calculateContextHeapSize(context, Long.MAX_VALUE, b);
102-
}
103-
10496
@Override
10597
protected synchronized void onCreate(Env env) {
10698
this.currentEnv = env;
@@ -202,15 +194,19 @@ Object call(Node node) {
202194
}
203195

204196
tracking.previousProperties = null;
205-
tracking.task = new ContextHeapSizeThreadLocalTask(tracking);
197+
tracking.task = new ContextHeapSizeThreadLocalTask(tracking, 1);
206198
// force update on start
207199
tracking.task.computeUpdate(true);
208-
tracking.timer = new Timer();
209-
tracking.timer.schedule(tracking.task, 0L, 1L);
200+
startWorkerThread(tracking);
210201

211202
return NullValue.NULL;
212203
}
213204
}
205+
206+
private void startWorkerThread(MemoryTracking tracking) {
207+
tracking.workerThread = currentEnv.createSystemThread(tracking.task);
208+
tracking.workerThread.start();
209+
}
214210
}
215211

216212
final class StopContextMemoryTrackingFunction extends BaseFunction {
@@ -223,12 +219,11 @@ Object call(Node node) {
223219
if (tracking.previousProperties != null) {
224220
return tracking.previousProperties;
225221
}
226-
if (tracking.timer == null) {
222+
if (tracking.task == null) {
227223
return NullValue.NULL;
228224
}
229-
tracking.task.cancel();
230-
tracking.timer.cancel();
231-
tracking.timer = null;
225+
226+
stopWorkerThread(node, tracking);
232227

233228
Map<String, Object> properties = new HashMap<>();
234229

@@ -243,12 +238,20 @@ Object call(Node node) {
243238

244239
// stop running actions for other threads
245240
tracking.previousProperties = new ReadOnlyProperties(properties);
246-
tracking.task.cancelled.set(true);
247241
tracking.task = null;
248242

249243
}
250244
return tracking.previousProperties;
251245
}
246+
247+
private void stopWorkerThread(Node node, MemoryTracking tracking) {
248+
// Disable the loop condition so the worker will exit.
249+
tracking.task.stop();
250+
// Wake the thread in case it's sleeping.
251+
tracking.workerThread.interrupt();
252+
// Wait until the thread has terminated.
253+
TruffleSafepoint.setBlockedThreadInterruptible(node, Thread::join, tracking.workerThread);
254+
}
252255
}
253256

254257
static class MemoryTracking {
@@ -257,14 +260,14 @@ static class MemoryTracking {
257260

258261
volatile ContextHeapSizeThreadLocalTask task;
259262
ReadOnlyProperties previousProperties;
260-
Timer timer;
263+
Thread workerThread;
261264

262265
MemoryTracking(TruffleContext context) {
263266
this.context = context;
264267
}
265268
}
266269

267-
final class ContextHeapSizeThreadLocalTask extends TimerTask {
270+
final class ContextHeapSizeThreadLocalTask implements Runnable {
268271

269272
final LongSummaryStatistics statistics = new LongSummaryStatistics();
270273
final AtomicBoolean cancelled = new AtomicBoolean();
@@ -276,14 +279,27 @@ final class ContextHeapSizeThreadLocalTask extends TimerTask {
276279

277280
long totalAllocatedMemory;
278281
final MemoryTracking tracking;
282+
final long intervalMillis;
279283

280-
ContextHeapSizeThreadLocalTask(MemoryTracking tracking) {
284+
ContextHeapSizeThreadLocalTask(MemoryTracking tracking, long intervalMillis) {
281285
this.tracking = tracking;
286+
this.intervalMillis = intervalMillis;
282287
}
283288

284289
@Override
285290
public void run() {
286-
computeUpdate(false);
291+
while (!cancelled.get()) {
292+
try {
293+
computeUpdate(false);
294+
Thread.sleep(intervalMillis);
295+
} catch (InterruptedException e) {
296+
break; // exit the loop if interrupted
297+
}
298+
}
299+
}
300+
301+
public void stop() {
302+
cancelled.set(true);
287303
}
288304

289305
void computeUpdate(boolean force) {

0 commit comments

Comments
 (0)