https://gradle.com/ logo
Join Slack
Powered by
This message was deleted.
# performance
s
This message was deleted.
c
Gradle does only create a single shared instance. Using different names or perhaps classloading fun (different class instances for the service) could result in different instances. How are your services registered?
➕ 1
👍 2
z
here's how we register ours: https://github.com/slackhq/slack-gradle-plugin/blob/95c8a5df9969d6f17259cd96969404[…]95/slack-plugin/src/main/kotlin/slack/gradle/SlackRootPlugin.kt, where that is only applied on the root project. We then consume it from many subprojects The service itself is here: https://github.com/slackhq/slack-gradle-plugin/blob/95c8a5df9969d6f17259cd96969404e80ff40c95/slack-plugin/src/main/kotlin/slack/gradle/SlackTools.kt We retrieve it as-needed from projects via these APIs https://github.com/slackhq/slack-gradle-plugin/blob/95c8a5df9969d6f17259cd96969404e80ff40c95/slack-plugin/src/main/kotlin/slack/gradle/SlackTools.kt#L170-L180 What we're seeing appears to be related to this: https://github.com/gradle/gradle/issues/17559. Which is quite tricky to manage because it seems to mean that any subproject with any difference of plugins on the classpath result in new, duplicate instances. A very trivial check of adding some static AtomicBoolean to check in the service's 
init
or registration creation action also shows that multiple instances are created.
c
hmmm. The parameters are odd as they include the project build directory (which makes no sense for a build-scoped service - this should perhaps be a parameter to a service method). Having different parameters for each registration is questionable.
z
the build directory is the root project's build directory, where this service is registered and where it writes build logs. Why is that odd?
c
it isn’t evident that is the case - 
project
is passed in as a parameter, so presumably it could be any project. Perhaps the code should be 
project.rootProject.layout.buildDirectory
.
z
but we only ever register it on the root project where that root plugin is applied. Why would it be created in other projects?
c
fair enough. the code is confusing, why pass in a project if it’s always the root one. But that isn’t the issue here.
not sure that linked issue is the case - it refers to failure modes relating to classloading / plugins, whereas your case is multiple registrations.
perhaps throw an exception on the second or higher registration to see the stack of what is tripping into there?
z
We saw pretty flatly that there were hundreds of instances for some reason, and seems to have started with gradle 8.0 or 8.1
tried the exception route but it comes from deep in gradle with no other traces to our plugin code 😕
c
understood. the stack may still be helpful if you have it handy.
z
let me get another one
👍 1
c
Looking at both your code and the Gradle code: • your code appears to only call the register once, from the root project • the Gradle registerIfAbsent checks solely off the name:
Copy code
BuildServiceRegistration<?, ?> existing = registrations.findByName(name);
            if (existing != null) {
                // TODO - assert same type
                // TODO - assert same parameters
                return uncheckedNonnullCast(existing.getService());
            }
Hard to see how that path is failing; perhaps something on the resolution / use of the service.
Hmmm. The plot thickens. The 
BuildServiceRegistry
(either injected or via project.gradle.sharedServices) is a Gradle-build singleton and stores a named set of services - one service instance per name, making it impossible to store multiple instances of a service (in a given BuildServiceRegistry). Perhaps somehow there are multiple BuildServiceRegistry instances…
z
my understanding is that build services are build-classpath-sensitive
so a partial attempt at mitigating this has been to put all plugins used in any project on the root buildscript classpath, even if they're not used in every project
c
yea. that pertains the the actual build service; the BuildServiceRegistry holding the services is a singleton itself, keyed by service name for registration / resolution, so not sensitive to the classpath as far as those operations go.
z
Could it re-init a new instance of a service if the classpath used for an existing one is changed?
c
that would be a bug if it’s occurring (it would cause this issue). the documented scope is across the entire Gradle build.
however, with the 
registerIfAbsent
only called once (other resolutions are lookups-by-name that fail if the name is not present and don’t create a service instance) it’s an unknown who would instantiate subsequent instances of the services. Having that stack (and perhaps one for the first - good - instantiation for comparison) may show us where in the Gradle internals its creating these.
This is the internals of the returned provider (what is resolved from the registration and returned at registration). It lazy-creates and memoizes the instance so subsequent .get() calls return the singleton service.
Copy code
private T getInstance() {
        listener.beforeGet(this);
        synchronized (this) {
            if (instance == null) {
                instance = instantiate();
            }
        }
        return instance.get();
    }
seems like this almost has to somehow be multiple BuildServiceRegistry instances.
z
for limiting the instances, should a static AtomicBoolean/AtomicInteger just be enough you think?
could miss if it's multiple classloaders though
c
we shouldn’t have to do that as Gradle purportedly does that for us. Something else is wrong here. What would be the steps to reproduce with that repo checked out?
z
I should be able to reproduce it in another public project, give me a min
👍 1
or rather let me cut a new release first with extra logging
👍 1
here we go: https://github.com/ZacSweers/CatchUp/pull/653
Copy code
~/dev/android/personal/CatchUp   z/testBuildServicesIssues ±  gw :app:assembleDebug --debug | grep SlackTools
2023-04-22T16:11:47.453-0400 [DEBUG] [SlackTools] [SlackTools] SlackTools created
2023-04-22T16:11:47.453-0400 [DEBUG] [SlackTools] [SlackTools] Multiple instances of SlackTools created. This is likely a bug in the build. New count is 3
c
sweet 👀
z
going to cut another release that logs the trace with the debug message
👍 1
c
ok, can see that debug output:
Copy code
➜  CatchUp git:(z/testBuildServicesIssues) ./gradlew :app:assembleDebug --debug | grep SlackTools
2023-04-22T13:21:02.356-0700 [DEBUG] [SlackTools] [SlackTools] SlackTools created
2023-04-22T13:21:02.356-0700 [DEBUG] [SlackTools] [SlackTools] Multiple instances of SlackTools created. This is likely a bug in the build. New count is 7
z
updated with the version that logs a trace
Copy code
2023-04-22T16:35:12.483-0400 [DEBUG] [SlackTools] [SlackTools] SlackTools created
2023-04-22T16:35:12.483-0400 [DEBUG] [SlackTools] [SlackTools] Multiple instances of SlackTools created. This is likely a bug in the build. New count is 27
java.lang.Throwable
        at slack.gradle.SlackTools.<init>(SlackTools.kt:83)
        at slack.gradle.SlackTools$Inject.<init>(Unknown Source)
        at java.base/jdk.internal.reflect.DirectConstructorHandleAccessor.newInstance(DirectConstructorHandleAccessor.java:67)
        at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:500)
        at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:484)
        at org.gradle.internal.instantiation.generator.AsmBackedClassGenerator$InvokeConstructorStrategy.newInstance(AsmBackedClassGenerator.java:1993)
        at org.gradle.internal.instantiation.generator.AbstractClassGenerator$GeneratedClassImpl$GeneratedConstructorImpl.newInstance(AbstractClassGenerator.java:512)
        at org.gradle.internal.instantiation.generator.DependencyInjectingInstantiator.doCreate(DependencyInjectingInstantiator.java:64)
        at org.gradle.internal.instantiation.generator.DependencyInjectingInstantiator.newInstance(DependencyInjectingInstantiator.java:55)
        at org.gradle.api.services.internal.RegisteredBuildServiceProvider.instantiate(RegisteredBuildServiceProvider.java:142)
        at org.gradle.api.services.internal.RegisteredBuildServiceProvider.instantiate(RegisteredBuildServiceProvider.java:131)
        at org.gradle.api.services.internal.RegisteredBuildServiceProvider.getInstance(RegisteredBuildServiceProvider.java:118)
        at org.gradle.api.services.internal.RegisteredBuildServiceProvider.calculateOwnValue(RegisteredBuildServiceProvider.java:111)
        at org.gradle.api.internal.provider.AbstractMinimalProvider.calculateOwnPresentValue(AbstractMinimalProvider.java:73)
        at org.gradle.api.internal.provider.AbstractMinimalProvider.get(AbstractMinimalProvider.java:93)
        at slack.gradle.SlackRootPlugin$configureRootProject$3$1$1.execute(SlackRootPlugin.kt:117)
        at slack.gradle.SlackRootPlugin$configureRootProject$3$1$1.execute(SlackRootPlugin.kt:116)
        at com.gradle.enterprise.gradleplugin.internal.extension.a$4.run(SourceFile:172)
        at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:577)
        at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:317)
        at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144)
        at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642)
        at java.base/java.lang.Thread.run(Thread.java:1589)
but I only see that logged once, which makes me think the static variable isn't quite the right approach here. Guessing it's persisting in the daemon
c
yea, the static variable will persist due to the daemon
z
actually I do see it multiple times
once at the beginning and once at the end
c
if all of them are that stack - using 
RegisteredBuildServiceProvider.calculateOwnValue
- those are the .get() calls to the provider that should be memoized
z
this is the first one, near the beginning of the build and with a much longer trace
the other one, earlier in the build.txt
c
run with 
--no-daemon
to sidestep caching
z
I think the odd thing here is this
Copy code
at com.gradle.enterprise.gradleplugin.internal.extension.a$4.run(SourceFile:172)
in the later one. It's as if the gradle enterprise plugin is re-initting it
c
that first one is from the registration in the root plugin that get()’s from the provider. All good there.
oh, intersting…
z
iiinteresting
and I bet it's re-initting becauase the build has already finished and probably closed the service (it implements 
AutoCloseable
)
c
yep. that would do it.
z
and I bet those don't get cleared out since it's after the services are closed?
c
yea, that’s after the services have been closed, so they’ll be orphaned.
don’t think we can fix that lifecycle. however, you could have SlackTools write the results <somewhere> on close and read those back in the GE handler.
alternately, and perhaps simpler, looks like you could pass SlackTools some object to populate on close, and also drop that object in a Gradle property to use in buildFinished.
z
I think the file is the route I have to go
that 
background
thing is tricky because it serializes all the inputs
c
ah yea.
z
now back to the original question heh
I wonder if there's a better way to track and limit the number of instances
maybe a lock file that's deleted in close?
c
Hmmm. Do we think that GE call after close was the sole place creating the extra instances? With that resolved are we back to a single instance?
z
seems like it
but I wanna lock it down to be sure
c
Not sure how we could do that. Anything static is persisted in the daemon across builds which will cause false positives.
z
I'm using a lock file
ish
ended up reverting that because, lo-and-behold, it somehow broke configuration caching
but otherwise things all working now
thanks for rubber ducking this with me, this was super helpful 👍
👍 1
2 Views
Open in Slack
PreviousNext
Powered by