Should Quarkus Gradle plugin avoid resolving effective config during configuration?

[cdsap]

Some of the Quarkus Gradle plugin tasks in this repository are configuration-cache compatible, and we are seeing cache hits for tasks like quarkusBuild. However, we have noticed that the configuration cache is invalidated when unrelated system properties change, even when those properties do not affect the Quarkus build (issue #52464).
In other words, the task outcomes do not change because these system properties are not actual inputs to the build, but the mere fact that they are read during configuration causes configuration-cache misses. Ideally, we should avoid reading system properties during configuration, and instead read them during task execution. If some system properties are actually influential, we should explicitly model them as inputs (similar to what we do for environment variables and Quarkus properties).

Where the system properties are being read
The first thing to understand is who is resolving the system properties that trigger invalidation. After some investigation, we observed this happens when instantiating SmallRyeConfigBuilder in ConfigUtils. During builder initialization, we call addDefaultSources() from SmallRye, and that’s where system properties are read:

protected List<ConfigSource> getDefaultSources() {
  List<ConfigSource> defaultSources = new ArrayList();
  defaultSources.addAll(this.getSystemSources());
  defaultSources.addAll(this.getPropertiesSources());
  return defaultSources;
}

This SmallRyeConfigBuilder is used to instantiate the effective configuration in the Quarkus Gradle plugin. Now that we know when the system-property read occurs, the remaining question is where in the plugin we are referencing configuration during Gradle configuration time.

Places where we found configuration-time reads
So far, we have identified the following locations:

1. QuarkusBuild: This task defines an @InputFiles property, getBuildInputFiles, which represents the “codegen output files” inputs for QuarkusBuild. The problem is that these inputs are currently computed during configuration and depend on general/package settings from the Quarkus extension. When we made this task configuration-cache compatible, we had to remove direct extension references and instead define explicit inputs, but some paths still trigger config resolution early. Also this fact could be related to issues like 9400d90
2. Task configuration blocks (QuarkusGenerateCode, QuarkusBuild, etc.) Some configuration blocks call methods that resolve the effective config to compute values, which triggers system property reads during configuration. This is caused the previous point.
3. QuarkusExtensionView: Reading values like ignoreEntries from the extension inside the constructor forces a new instantiation of the build config, which again triggers effective config creation.

Potential solutions

1. Ignore system properties in config resolution: In theory, Quarkus Gradle configuration could define a set of system properties to ignore. However, this behavior is inside SmallRye (SysPropConfigSource), so unless we can influence that source behavior, this seems unavoidable once we instantiate the effective configuration
2. Remove configuration-time BaseConfig/effective-config calls in the problematic paths
   This would mean restructuring those parts of the plugin so that effective config is only instantiated during execution, and any required values are either:

• read lazily during execution, or
• modeled explicitly as task inputs

If we proceed with option 2, we will likely need to redefine QuarkusBuild#getBuildInputFiles. This code is old, but it is critical because it directly affects the task inputs of the Quarkus build. I think this needs an open discussion before changing it.

Side note

Because these configuration-cache misses are impacting us, I prepared a PR that introduces an experimental mode which disables using BuildConfig during configuration time. This allows full configuration-cache compatibility in cases where the cache would otherwise miss due to unrelated system property changes.

[aloubyansky]

Thanks for looking into this @cdsap

If we proceed with option 2, we will likely need to redefine QuarkusBuild#getBuildInputFiles. This code is old, but it is critical because it directly affects the task inputs of the Quarkus build. I think this needs an open discussion before changing it.

FWIW, I don't see for what getBuildInputFiles() is actually used.

[cdsap]

hi @aloubyansky,
The task declares this input files via:

@InputFiles
@PathSensitive(PathSensitivity.RELATIVE)
protected Collection<File> getBuildInputFiles() {
    List<File> inputs = new ArrayList<>();
    if (nativeEnabled()) {
        if (jarEnabled()) {
            throw nativeAndJar();
        }
        if (nativeSourcesOnly()) {
            // nothing
        } else {
            Path appBuildBaseDir = appBuildDir();
            inputs.add(genBuildDir().toFile());
            inputs.add(appBuildBaseDir.resolve(outputDirectory()).toFile());
            runnerAndArtifactsInputs(inputs::add, appBuildBaseDir);
        }
    } else if (jarEnabled()) {
        PackageConfig.JarConfig.JarType packageType = jarType();
        switch (packageType) {
            case FAST_JAR, AOT_JAR -> {
                Path appBuildBaseDir = appBuildDir();
                inputs.add(genBuildDir().toFile());
                inputs.add(appBuildBaseDir.resolve(outputDirectory()).toFile());
                runnerAndArtifactsInputs(inputs::add, appBuildBaseDir);
            }
            case LEGACY_JAR -> {
                inputs.add(depBuildDir().resolve("lib").toFile());
                inputs.add(appBuildDir().resolve("lib").toFile());
                runnerAndArtifactsInputs(inputs::add, appBuildDir());
            }
            case MUTABLE_JAR, UBER_JAR -> {
                // no additional inputs
            }
        }
    }
    return inputs;
}

Even though this method is not explicitly referenced from build logic like task.getBuildInputFiles().from(...), it is still evaluated during configuration as part of task input snapshotting. That is the issue, because computing the input list requires resolving values from the BaseConfig, for example nativeEnabled(), jarEnabled(), and jarType().

This means the set of inputs is dynamically computed based on packaging mode. For example, when jarType is FAST_JAR, the inputs include genBuildDir and the outputDirectory under appBuildDir. When it is LEGACY_JAR, the task also includes the lib directories under both depBuildDir and appBuildDir. So the task inputs depend on runtime configuration decisions, and those decisions require reading configuration values during configuration time.

If we rework this, we can establish clearer boundaries for QuarkusBuild inputs and reduce configuration time reads. I considered using only QUARKUS_BUILD_APP_DIR as a single input, but given the conditional logic above, we would need to confirm that it always covers all relevant directories (especially the lib directories for LEGACY_JAR, and any runner/artifacts inputs added by runnerAndArtifactsInputs).

[dmlloyd]

Where the system properties are being read The first thing to understand is who is resolving the system properties that trigger invalidation. After some investigation, we observed this happens when instantiating SmallRyeConfigBuilder in ConfigUtils. During builder initialization, we call addDefaultSources() from SmallRye, and that’s where system properties are read:

@radcortez do we need a system property config source under Maven and Gradle? Aren't we bringing in the build system properties later, which serve the same purpose?

Another thought is that maybe we can filter down the system properties source to exclude anything outside of quarkus.* under Maven and Gradle.

[radcortez]

Unsure... The big PR #31166 that should have fixed Gradle cache includes System Properties as a source, but I can't tell is on purpose or the described issue was not considered.

I don't have enough knowledge on how Gradle cache works, but I find it strange that just reading the System Config Source in EffectiveConfig causes issues with the Gradle cache. I would need to check on what is actually happening there.

[aloubyansky]

@radcortez would reading systems properties lazily be an option during the config initialization?

[aloubyansky]

It seems like we could try initializing EffectiveConfig values and quarkusValues lazily instead of in the constructor.

[radcortez]

Yes, that can be done. Is that enough for Gradle?

[aloubyansky]

No, it doesn't look like it is.

[radcortez]

What I can see is that the full list of configurations is passed down to the Task input getCachingRelevantInput.

We do need the full list to propagate the configuration to the Gradle Workers, and we have a list that contains all of the properties and another with only quarkus properties. I'm unsure when to use one of the other. For reference, the one with only quarkus properties was added in #48496. I know this is not the exact issue, but I'm wondering about reducing the lists we use over there. I guess using the full list was just an oversight, convenience, or things changed so much that now we have this issue.

I was thinking that we should introduce a list of properties to cache instead of the full list, but that will be a third list :) We could just pass in the names from PackageConfig and NativeConfig.

[radcortez]

@aloubyansky any thoughts?