Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The 'threading' CLI option/build param/target property changed to 'single threaded' #1817

Merged
merged 28 commits into from
Nov 8, 2023
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
80c5637
Changed the 'threading' CLI option, build param and target property t…
patilatharva Jun 4, 2023
937ea82
Rename ThreadingProperty to SingleThreadedProperty and invert uses
lhstrh Nov 3, 2023
9d4b8de
Changed property in LF tests.
patilatharva Jun 7, 2023
3d3f064
Adjust description
lhstrh Nov 3, 2023
1bec637
Made CLI options --single-threaded and --workers mutually exclusive.
patilatharva Jul 3, 2023
4486ab0
Added LF validation test for mutually exclusive threading properties.
patilatharva Jul 3, 2023
6380e24
Minor changes
lhstrh Nov 3, 2023
7c1e56b
Fix unit tests and improve validation mechanism
lhstrh Nov 5, 2023
a299194
Fix bug
lhstrh Nov 5, 2023
9e7eecc
Fix another logic problem
lhstrh Nov 5, 2023
95d9e69
Merge branch 'master' into single-threaded
lhstrh Nov 5, 2023
7b9c33b
Comments
lhstrh Nov 5, 2023
9564ada
Fix bug in FederateTargetConfig
lhstrh Nov 5, 2023
98a06ba
Modulo some code duplication, this should take care of the path problem
lhstrh Nov 6, 2023
bb72e52
Cleanup
lhstrh Nov 6, 2023
0ece338
Comment
lhstrh Nov 6, 2023
00f71f1
Address comment from @cmnrd
lhstrh Nov 6, 2023
c26d630
Update core/src/main/java/org/lflang/generator/c/CGenerator.java
lhstrh Nov 6, 2023
34498d5
Fix Arduino building
erlingrj Nov 6, 2023
91c36dd
Code-generate LF_SINGLE_THREADED also
erlingrj Nov 6, 2023
00dc9a2
Bump reactor-c
erlingrj Nov 6, 2023
f3aaf0b
Fix Arduino compilation
erlingrj Nov 6, 2023
f0d761e
Spotless
erlingrj Nov 6, 2023
daaee0c
Merge branch 'master' into single-threaded
erlingrj Nov 6, 2023
7611bed
Merge branch 'master' into single-threaded
erlingrj Nov 7, 2023
3132c4e
Bump reactor-c
erlingrj Nov 7, 2023
79ba285
Merge remote-tracking branch 'origin/master' into single-threaded
erlingrj Nov 7, 2023
e58de5b
Update submodule
lhstrh Nov 8, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 35 additions & 18 deletions cli/lfc/src/main/java/org/lflang/cli/Lfc.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
import org.lflang.target.property.PrintStatisticsProperty;
import org.lflang.target.property.RuntimeVersionProperty;
import org.lflang.target.property.SchedulerProperty;
import org.lflang.target.property.ThreadingProperty;
import org.lflang.target.property.SingleThreadedProperty;
import org.lflang.target.property.TracingProperty;
import org.lflang.target.property.TracingProperty.TracingOptions;
import org.lflang.target.property.VerifyProperty;
Expand All @@ -33,6 +33,7 @@
import org.lflang.target.property.type.LoggingType.LogLevel;
import org.lflang.target.property.type.SchedulerType;
import org.lflang.target.property.type.SchedulerType.Scheduler;
import picocli.CommandLine.ArgGroup;
import picocli.CommandLine.Command;
import picocli.CommandLine.Option;

Expand Down Expand Up @@ -139,22 +140,28 @@ public class Lfc extends CliBase {
description = "Specify the runtime scheduler (if supported).")
private String scheduler;

@Option(
names = {"-t", "--threading"},
paramLabel = "<true/false>",
description = "Specify whether the runtime should use multi-threading" + " (true/false).")
private String threading;

@Option(
names = {"--tracing"},
arity = "0",
description = "Specify whether to enable run-time tracing (if supported).")
private Boolean tracing;

@Option(
names = {"-w", "--workers"},
description = "Specify the default number of worker threads.")
private Integer workers;
/** Mutually exclusive options related to threading. */
static class ThreadingMutuallyExclusive {
@Option(
names = "--single-threaded",
arity = "0",
description = "Specify whether the runtime should be single-threaded.")
patilatharva marked this conversation as resolved.
Show resolved Hide resolved
private boolean singleThreaded;

@Option(
names = {"-w", "--workers"},
description = "Specify the number of worker threads.")
private Integer workers;
}

@ArgGroup(exclusive = true, multiplicity = "0..1")
ThreadingMutuallyExclusive threading;

/**
* Main function of the stand-alone compiler. Caution: this will invoke System.exit.
Expand Down Expand Up @@ -331,15 +338,25 @@ private TracingOptions getTracingOptions() {
}
}

/** Return whether threading has been enabled via the CLI arguments, or {@code null} otherwise. */
private Boolean getThreading() {
/** Return the single threaded mode has been specified, or {@code null} if none was specified. */
private Boolean getSingleThreaded() {
Boolean singleThreaded = null;
// Set one of the mutually-exclusive threading options.
if (threading != null) {
return Boolean.parseBoolean(threading);
} else {
return null;
singleThreaded = threading.singleThreaded;
}
return singleThreaded;
}

/** Return the number of workers specified, or {@code null} if none was specified. */
private Integer getWorkers() {
Integer workers = null;
// Set one of the mutually-exclusive threading options.
if (threading != null) {
workers = threading.workers;
}
return workers;
}
/** Check the values of the commandline arguments and return them. */
public GeneratorArguments getArgs() {

Expand All @@ -360,8 +377,8 @@ public GeneratorArguments getArgs() {
new Argument<>(VerifyProperty.INSTANCE, verify),
new Argument<>(RuntimeVersionProperty.INSTANCE, runtimeVersion),
new Argument<>(SchedulerProperty.INSTANCE, getScheduler()),
new Argument<>(ThreadingProperty.INSTANCE, getThreading()),
new Argument<>(SingleThreadedProperty.INSTANCE, getSingleThreaded()),
new Argument<>(TracingProperty.INSTANCE, getTracingOptions()),
new Argument<>(WorkersProperty.INSTANCE, workers)));
new Argument<>(WorkersProperty.INSTANCE, getWorkers())));
}
}
22 changes: 12 additions & 10 deletions cli/lfc/src/test/java/org/lflang/cli/LfcCliTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,8 @@
import org.lflang.target.property.PrintStatisticsProperty;
import org.lflang.target.property.RuntimeVersionProperty;
import org.lflang.target.property.SchedulerProperty;
import org.lflang.target.property.SingleThreadedProperty;
import org.lflang.target.property.TargetProperty;
import org.lflang.target.property.ThreadingProperty;
import org.lflang.target.property.WorkersProperty;
import org.lflang.target.property.type.BuildTypeType.BuildType;
import org.lflang.target.property.type.LoggingType.LogLevel;
import org.lflang.target.property.type.SchedulerType.Scheduler;
Expand Down Expand Up @@ -92,8 +91,7 @@ public class LfcCliTest {
"rti": "path/to/rti",
"runtime-version": "rs",
"scheduler": "GEDF_NP",
"threading": false,
"workers": "1"
"single-threaded": true
}
}
""";
Expand Down Expand Up @@ -136,6 +134,14 @@ public void testMutuallyExclusiveCliArgs() {
result.checkStdErr(containsString("are mutually exclusive (specify only one)"));
result.checkFailed();
});

lfcTester
.run("File.lf", "--single-threaded", "--workers", "1")
.verify(
result -> {
result.checkStdErr(containsString("are mutually exclusive (specify only one)"));
result.checkFailed();
});
}

@Test
Expand Down Expand Up @@ -265,8 +271,7 @@ public void verifyGeneratorArgs(Path tempDir, String[] args) {
checkOverrideValue(genArgs, PrintStatisticsProperty.INSTANCE, true);
checkOverrideValue(genArgs, RuntimeVersionProperty.INSTANCE, "rs");
checkOverrideValue(genArgs, SchedulerProperty.INSTANCE, Scheduler.GEDF_NP);
checkOverrideValue(genArgs, ThreadingProperty.INSTANCE, false);
checkOverrideValue(genArgs, WorkersProperty.INSTANCE, 1);
checkOverrideValue(genArgs, SingleThreadedProperty.INSTANCE, true);

assertEquals(true, genArgs.clean());
assertEquals("src", Path.of(genArgs.externalRuntimeUri()).getFileName().toString());
Expand Down Expand Up @@ -333,10 +338,7 @@ public void testGeneratorArgs(@TempDir Path tempDir) throws IOException {
"rs",
"--scheduler",
"GEDF_NP",
"--threading",
"false",
"--workers",
"1",
"--single-threaded"
};
verifyGeneratorArgs(tempDir, args);
}
Expand Down
19 changes: 17 additions & 2 deletions core/src/main/java/org/lflang/ast/ASTUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -151,10 +151,10 @@ public static List<Reactor> getAllReactors(Resource resource) {
}

/**
* Get the main reactor defined in the given resource.
* Get the main reactor defined in the given resource, if there is one.
*
* @param resource the resource to extract reactors from
* @return An iterable over all reactors found in the resource
* @return An {@code Optional} reactor that may be present or absent.
*/
public static Optional<Reactor> getMainReactor(Resource resource) {
return StreamSupport.stream(
Expand All @@ -165,6 +165,21 @@ public static Optional<Reactor> getMainReactor(Resource resource) {
.findFirst();
}

/**
* Get the federated reactor defined in the given resource, if there is one.
*
* @param resource the resource to extract reactors from
* @return An {@code Optional} reactor that may be present or absent.
*/
public static Optional<Reactor> getFederatedReactor(Resource resource) {
return StreamSupport.stream(
IteratorExtensions.toIterable(resource.getAllContents()).spliterator(), false)
.filter(Reactor.class::isInstance)
.map(Reactor.class::cast)
.filter(it -> it.isFederated())
.findFirst();
}

/**
* Find connections in the given resource that would be conflicting writes if they were not
* located in mutually exclusive modes.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@
import org.lflang.target.property.CoordinationProperty;
import org.lflang.target.property.FedSetupProperty;
import org.lflang.target.property.KeepaliveProperty;
import org.lflang.target.property.ThreadingProperty;
import org.lflang.target.property.SingleThreadedProperty;
import org.lflang.target.property.type.CoordinationModeType.CoordinationMode;

/**
Expand Down Expand Up @@ -93,7 +93,7 @@ public void initializeTargetConfig(
// Also, create the RTI C file and the launcher script.
// Handle target parameters.
// If the program is federated, then ensure that threading is enabled.
ThreadingProperty.INSTANCE.override(federate.targetConfig, true);
SingleThreadedProperty.INSTANCE.override(federate.targetConfig, false);

// Include the fed setup file for this federate in the target property
FedSetupProperty.INSTANCE.override(federate.targetConfig, getPreamblePath(federate));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -291,9 +291,7 @@ private Map<Path, CodeMap> compileFederates(

TargetConfig subConfig =
new TargetConfig(
GeneratorUtils.findTargetDecl(subFileConfig.resource),
GeneratorArguments.none(),
subContextMessageReporter);
subFileConfig.resource, GeneratorArguments.none(), subContextMessageReporter);
if (targetConfig.get(DockerProperty.INSTANCE).enabled
&& targetConfig.target.buildsUsingDocker()) {
NoCompileProperty.INSTANCE.override(subConfig, true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@
import org.lflang.generator.GeneratorUtils;
import org.lflang.generator.LFGeneratorContext;
import org.lflang.lf.KeyValuePair;
import org.lflang.lf.LfFactory;
import org.lflang.target.Target;
import org.lflang.target.TargetConfig;
import org.lflang.target.property.ClockSyncModeProperty;
import org.lflang.target.property.ClockSyncOptionsProperty;
import org.lflang.target.property.FileListProperty;
import org.lflang.util.FileUtil;

/**
Expand All @@ -34,20 +34,27 @@ public class FederateTargetConfig extends TargetConfig {
* @param federateResource The resource in which to find the reactor class of the federate.
*/
public FederateTargetConfig(LFGeneratorContext context, Resource federateResource) {
// Create target config based on the main .lf file (but with the target of the federate,
// which could be different).
super(
Target.fromDecl(GeneratorUtils.findTargetDecl(federateResource)),
GeneratorUtils.findTargetDecl(context.getFileConfig().resource).getConfig(),
context.getArgs(),
context.getErrorReporter());
// Create target config with the target based on the federate (not the main resource).
super(Target.fromDecl(GeneratorUtils.findTargetDecl(federateResource)));
var federationResource = context.getFileConfig().resource;
var reporter = context.getErrorReporter();

mergeImportedConfig(
federateResource, context.getFileConfig().resource, context.getErrorReporter());
this.mainResource = federationResource;

// Load properties from the main file
load(federationResource, reporter);

// Load properties from the federate file
mergeImportedConfig(federateResource, federationResource, reporter);

// Load properties from the generator context
load(context.getArgs(), reporter);

clearPropertiesToIgnore();

((FederationFileConfig) context.getFileConfig()).relativizePaths(this);

this.validate(reporter);
}

/**
Expand Down Expand Up @@ -98,28 +105,22 @@ private void clearPropertiesToIgnore() {
*/
public void update(
TargetConfig config, List<KeyValuePair> pairs, Path relativePath, MessageReporter err) {
// FIXME: https://issue.lf-lang.org/2080
pairs.forEach(
pair -> {
var p = config.forName(pair.getName());
if (p.isPresent()) {
var value = pair.getValue();
if (pair.getName().equals("files")) {
var array = LfFactory.eINSTANCE.createArray();
ASTUtils.elementToListOfStrings(pair.getValue()).stream()
.map(relativePath::resolve) // assume all paths are relative
.map(Objects::toString)
.map(
s -> {
var element = LfFactory.eINSTANCE.createElement();
element.setLiteral(s);
return element;
})
.forEach(array.getElements()::add);
value = LfFactory.eINSTANCE.createElement();
value.setArray(array);
var property = p.get();
if (property instanceof FileListProperty fileListProperty) {
var files =
ASTUtils.elementToListOfStrings(value).stream()
.map(relativePath::resolve) // assume all paths are relative
.map(Objects::toString)
.toList();
fileListProperty.update(config, files);
} else {
p.get().update(this, pair, err);
}
p.get().update(this, value, err);
}
});
}
Expand Down
6 changes: 4 additions & 2 deletions core/src/main/java/org/lflang/generator/GeneratorBase.java
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@
import org.lflang.target.Target;
import org.lflang.target.TargetConfig;
import org.lflang.target.property.FilesProperty;
import org.lflang.target.property.ThreadingProperty;
import org.lflang.target.property.SingleThreadedProperty;
import org.lflang.target.property.VerifyProperty;
import org.lflang.util.FileUtil;
import org.lflang.validation.AbstractLFValidator;
Expand Down Expand Up @@ -267,7 +267,9 @@ public void doGenerate(Resource resource, LFGeneratorContext context) {

// Check for the existence and support of watchdogs
hasWatchdogs = IterableExtensions.exists(reactors, it -> !it.getWatchdogs().isEmpty());
checkWatchdogSupport(getTarget() == Target.C && targetConfig.get(ThreadingProperty.INSTANCE));

checkWatchdogSupport(
getTarget() == Target.C && !targetConfig.get(SingleThreadedProperty.INSTANCE));
additionalPostProcessingForModes();
}

Expand Down
3 changes: 1 addition & 2 deletions core/src/main/java/org/lflang/generator/GeneratorUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import org.lflang.lf.KeyValuePairs;
import org.lflang.lf.Reactor;
import org.lflang.lf.TargetDecl;
import org.lflang.target.Target;
import org.lflang.target.TargetConfig;
import org.lflang.target.property.KeepaliveProperty;

Expand Down Expand Up @@ -120,7 +119,7 @@ public static LFResource getLFResource(
MessageReporter messageReporter) {
var target = ASTUtils.targetDecl(resource);
KeyValuePairs config = target.getConfig();
var targetConfig = new TargetConfig(Target.fromDecl(target));
var targetConfig = new TargetConfig(resource, context.getArgs(), messageReporter);
if (config != null) {
List<KeyValuePair> pairs = config.getPairs();
targetConfig.load(pairs != null ? pairs : List.of(), messageReporter);
Expand Down
3 changes: 1 addition & 2 deletions core/src/main/java/org/lflang/generator/MainContext.java
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,6 @@ public void reportProgress(String message, int percentage) {
* in the target configuration.
*/
public void loadTargetConfig() {
this.targetConfig =
new TargetConfig(GeneratorUtils.findTargetDecl(fileConfig.resource), args, messageReporter);
this.targetConfig = new TargetConfig(fileConfig.resource, args, messageReporter);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
import org.lflang.target.property.CompilerProperty;
import org.lflang.target.property.PlatformProperty;
import org.lflang.target.property.ProtobufsProperty;
import org.lflang.target.property.ThreadingProperty;
import org.lflang.target.property.SingleThreadedProperty;
import org.lflang.target.property.WorkersProperty;
import org.lflang.target.property.type.PlatformType.Platform;
import org.lflang.util.FileUtil;
Expand Down Expand Up @@ -354,7 +354,7 @@ CodeBuilder generateCMakeCode(
cMakeCode.newLine();
}

if (targetConfig.get(ThreadingProperty.INSTANCE)
if (!targetConfig.get(SingleThreadedProperty.INSTANCE)
&& platformOptions.platform() != Platform.ZEPHYR) {
// If threaded computation is requested, add the threads option.
cMakeCode.pr("# Find threads and link to it");
Expand All @@ -365,7 +365,7 @@ CodeBuilder generateCMakeCode(

// Add additional flags so runtime can distinguish between multi-threaded and single-threaded
// mode
if (targetConfig.get(ThreadingProperty.INSTANCE)) {
if (!targetConfig.get(SingleThreadedProperty.INSTANCE)) {
cMakeCode.pr("# Set the number of workers to enable threading/tracing");
cMakeCode.pr(
"target_compile_definitions(${LF_MAIN_TARGET} PUBLIC NUMBER_OF_WORKERS="
Expand Down
Loading
Loading