search

eclipse-jdt / eclipse.jdt.core

Eclipse Public License 2.0
157 stars 124 forks source link

ephemeral memory allocation in JRTUtil.getJrtSystem() #2815

Open jukzi opened 1 month ago

jukzi commented 1 month ago

Currently JDK Mission Control reports JRTUtil.getJrtSystem() as top most memory allocator during full build

caused by https://github.com/eclipse-jdt/eclipse.jdt.core/pull/2370 see also https://github.com/eclipse-jdt/eclipse.jdt.core/pull/2394#issuecomment-2109776599 ff

image

HannesWell commented 1 month ago

caused by #2370

How can a PR that was not submitted cause a regression? :P I guess you mean https://github.com/eclipse-jdt/eclipse.jdt.core/pull/2376, don't you?

see also #2394 (comment) ff

As written in the ff, if possible it would be best to save a reference to the file-system at an appropriated place. This would save also the look-ups or at least use Path all the time.

In general it would be good to mention the build you monitored, this allows other to reproduce the results. One of the Eclipse top-level projects would of course be handy, maybe even the entire SDK.

But from the screenshot you provided I read that org.eclipse.jdt.internal.compiler.util.lJdk.toJdkHome() is the main cause isn't it? That method calls Path.getParent() twice since its inception in https://github.com/eclipse-jdt/eclipse.jdt.core/pull/217. So it doesn't look like a regression.

jukzi commented 1 month ago

There was a chain of multiple PRs in that area.... My testcase was simply setting up a workspace as descripted in https://github.com/eclipse-platform/.github/blob/main/CONTRIBUTING.md and measuring Project/clean.../"Clean all projects".

@HannesWell do you plan to work on it? Then i would assign it to you. Otherwise i would just leave this open as a reminder that there is room for improvement. It's not urgent but JDT is much optimized for avoiding memory allocations and it's absurd that now this code allocates more memory then all the sourcecodes compiled.

So it doesn't look like a regression.

I trust in measurements more then in speculations.

HannesWell commented 1 month ago

@HannesWell do you plan to work on it? Then i would assign it to you. Otherwise i would just leave this open as a reminder that there is room for improvement. It's not urgent but JDT is much optimized for avoiding memory allocations and it's absurd that now this code allocates more memory then all the sourcecodes compiled.

I don't plan to work on this immediately, but maybe in the future. I'm fine if this is just kept open as a reminder, if I work on this, I'll let you know.

So it doesn't look like a regression.

I trust in measurements more then in speculations.

Why is analyzing the git-logs and concluding that the methods that you identified in your measurements did not change recently speculation?