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

Heap usage regression in 8.3-rc-1 #25834

Closed
erdi opened this issue Jul 20, 2023 · 19 comments
Closed

Heap usage regression in 8.3-rc-1 #25834

erdi opened this issue Jul 20, 2023 · 19 comments
Assignees
Milestone

Comments

@erdi
Copy link
Contributor

erdi commented Jul 20, 2023

Expected Behavior

8.3-rc-1 does not use significantly more heap space memory.

Current Behavior

A build invocation which passes on 8.2.1 fails with java.lang.OutOfMemoryError: Java heap space on 8.3-rc-1.

Context (optional)

This is an early heads up that there seems to be a heap space usage regression in 8.3-rc-1. I need to do some more digging but from what I've seen so far on our fairly large build (1100+ projects) it seems to be related to resolving of a lot of dependency configurations when tasks contributed by com.github.jk1.dependency-license-report plugin are executed.

The main point is that executing the exact same tasks on 8.2.1 does not throw OutOfMemoryError but it does on 8.3-rc-1.

I will take some heap dumps and compare them between the versions in an effort to try to pin point what it is being kept in memory on 8.3-rc-1.

Steps to Reproduce

Currently unknown.

Gradle version

8.3-rc-1

Gradle version that used to work

8.2.1

Build scan URL (optional)

No response

Your Environment (optional)

No response

@erdi erdi added a:regression This used to work to-triage labels Jul 20, 2023
@ov7a
Copy link
Member

ov7a commented Jul 20, 2023

@erdi Thank you for the heads up!

You can use Gradle Project Replicator to reproduce the structure of your project and see whether the issue persists.

@ov7a ov7a added the pending:reproducer Indicates that the issue requires a reproducer or will be closed after 7 days label Jul 21, 2023
@erdi
Copy link
Contributor Author

erdi commented Jul 25, 2023

A small update on this, @ov7a.

I took some heap dumps using gradle-profiler and I can see 4 times as many DefaultConfiguration_Decorated instances retained on 8.3-rc-1 compared to 8.2.1, all of them referenced via DetachedConfigurationsProvider. The next step would be to verify if these detached configurations are resolved on both versions and only retained in memory on 8.3-rc-1 or whether they are not even created on 8.2.1. I unfortunately cannot spend any more time on this at the moment but it will look into it again when I'm back at work next week.

@github-actions github-actions bot removed the pending:reproducer Indicates that the issue requires a reproducer or will be closed after 7 days label Jul 25, 2023
@ljacomet ljacomet added this to the 8.3 RC3 milestone Jul 27, 2023
@eskatos
Copy link
Member

eskatos commented Jul 27, 2023

Hi Marcin 👋
Would it be possible for you to share heap dumps? This would help us pinpoint the problem

@erdi
Copy link
Contributor Author

erdi commented Jul 27, 2023

I’m off until Tuesday so I unfortunately won’t be able to provide any more info before then as I don’t even have access to the project atm. It’s also highly unlikely that I will be able to provide you with full heap dumps as that poses a risk of leaking credentials that are being passed to the build. My current plan is to prioritise investigating this further as soon as I’m back at work so that I can provide feedback in as timely manner as possible.

Just FYI, it’s clearly a regression but there is a third party plugin involved so it might be just due to that plugin doing something silly. I will come back to you on Tuesday even if it will be just to let you know that I’ve not made any progress.

@erdi
Copy link
Contributor Author

erdi commented Aug 1, 2023

Do I understand correctly that you managed to track this issue down on your end with #25949 being a potential fix? Do you still need me to investigate and provide more info, @eskatos, @ljacomet, @jvandort?

FYI, I'm more than happy to test any nightlies with a potential fix as they become available, just let me know.

@jvandort
Copy link
Member

jvandort commented Aug 1, 2023

We have determined the root cause of holding references to the detached configurations, which the PR attempts to resolve. An optimization was added in 8.3 which cached all resolved components in a given build in memory. This would normally be acceptable, however detached configurations themselves create ad-hoc components which in this case causes the memory regression.

We are still looking into a proper solution to this, as the above linked PR will undo other important memory optimizations for non-detached-configuration situations.

@ljacomet
Copy link
Member

ljacomet commented Aug 1, 2023

@erdi Could you try increasing memory to have the build pass and communicate how much more is needed?

@erdi
Copy link
Contributor Author

erdi commented Aug 1, 2023

So rather than increasing memory I ran gradle-profiler on the subset of the project (projects in our build are grouped into domains, so I just used one group directory as the build project directory).

Heapdump stats for 8.2.1:

Total Bytes: 752,053,978
Total Classes: 31,367
Total Instances: 14,065,611
Classloaders: 2,210
GC Roots: 5,127

Heapdump stats for 8.3-rc-1:

Total Bytes: 1,645,578,458
Total Classes: 38,744
Total Instances: 31,717,262
Classloaders: 2,407
GC Roots: 4,157

Top classes by retained size on 8.2.1:
Screenshot 2023-08-01 at 21 01 30

Top classes by retained size on 8.3-rc-1:
Screenshot 2023-08-01 at 21 03 35

Hopefully that is helpful, let me know if there is any more info that I can provide.

@ljacomet
Copy link
Member

ljacomet commented Aug 2, 2023

Thank you for the additional information.
Given the work required to fix this regression with detached configurations, we have decided to accept it for 8.3 and provide a fix at the latest in Gradle 8.4.

Builds affected by this regression should stick with Gradle 8.2.1.

@erdi
Copy link
Contributor Author

erdi commented Aug 2, 2023

Thanks for the explanation. We will skip 8.3 and I will report back here if it has been fixed in 8.4 when an RC for that version is released.

@eboudrant
Copy link

eboudrant commented Aug 29, 2023

@ljacomet does this issue also affect Gradle 8.2.1? We're getting similar error when we run the DependencyReportTask:

Exception: java.lang.OutOfMemoryError thrown from the UncaughtExceptionHandler in thread "pool-1-thread-1"
Exception: java.lang.OutOfMemoryError thrown from the UncaughtExceptionHandler in thread "main"

And same error with Gradle 8.3.

If you think this is unrelated I can create a new issue.

@erdi
Copy link
Contributor Author

erdi commented Aug 29, 2023

This issue is about a regression in Gradle 8.3 so no, 8.2.1 is not affected by this particular issue, @eboudrant. If you're getting this on 8.2.1 then it sounds like you might want to allocate more heap space to Gradle.

@eboudrant
Copy link

Ok, I will open a new issue since the error I'm getting is with a heap already at 56G. Same OOM in 8.2.1 or 8.3.

@jvandort
Copy link
Member

This should have been fixed by #26134

@ljacomet
Copy link
Member

@erdi @eboudrant Could you test a recent 8.4 nightly and see if the memory usage is back to normal?

@eboudrant
Copy link

eboudrant commented Aug 29, 2023

Same error with 8.4-20230828223511+0000

Exception: java.lang.OutOfMemoryError thrown from the UncaughtExceptionHandler in thread "pool-1-thread-1"

I suspect something different since my heap is 56G but the OOM happen before it's exhausted, happen at about 10G usage.
image

I also wonder what is the thread "pool-1-thread-1", when I use Yourkit I can't find this thread.

@erdi
Copy link
Contributor Author

erdi commented Aug 29, 2023

@ljacomet, here's the heap summary from a dump taken using gradle-profiler on 8.2.1:

Total Bytes: 790,360,722
Total Classes: 29,665
Total Instances: 14,811,477
Classloaders: 2,242
GC Roots: 4,140

And here's the same for 8.4-20230828223511+0000

Total Bytes: 829,980,840
Total Classes: 40,224
Total Instances: 15,490,462
Classloaders: 2,443
GC Roots: 5,138

The heap size increase is minimal now (it was double) so I guess the regression has been fixed.

@ljacomet
Copy link
Member

ljacomet commented Aug 30, 2023

@erdi Thanks, let's consider this issue as resolved then.

@eboudrant Please file another issue, if you did not do it already, as it seems you are hitting a different problem.

@eboudrant
Copy link

@ljacomet it's a different problem, the OOM actually happen in the wrapper : #26228

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants