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

UpdateSitePublisher -addJREIU argument doesn't add current JVM #462

Merged
merged 1 commit into from
Feb 12, 2024

Conversation

iloveeclipse
Copy link
Member

Don't hard code "default" JRE version if we "guess one" and create some profile based on current JRE packages.
Just read & use the version from the running JRE.

Fixes #461

Copy link

github-actions bot commented Feb 12, 2024

Test Results

    9 files  ±0      9 suites  ±0   29m 52s ⏱️ - 4m 57s
2 183 tests ±0  2 179 ✅ ±0   4 💤 ±0  0 ❌ ±0 
6 639 runs  ±0  6 628 ✅ ±0  11 💤 ±0  0 ❌ ±0 

Results for commit b5f1716. ± Comparison against base commit 4327aa9.

♻️ This comment has been updated with latest results.

@iloveeclipse
Copy link
Member Author

Build fails with org.eclipse.equinox.p2.tests: Error assembling JAR: Problem creating jar: Execution exception: Java heap space.
is this something that changed recently?

@laeubi
Copy link
Member

laeubi commented Feb 12, 2024

Please add https://github.com/eclipse-platform/eclipse.platform.releng.aggregator/blob/master/.mvn/jvm.config to this repository as well, it should help with that OOM errors as currently only 1GB of 4G is used by default.

@iloveeclipse
Copy link
Member Author

Please add https://github.com/eclipse-platform/eclipse.platform.releng.aggregator/blob/master/.mvn/jvm.config to this repository as well, it should help with that OOM errors as currently only 1GB of 4G is used by default.

We surely won't have that workaround be forever, is there a ticket for that so I can link the PR against?

Don't hard code "default" JRE version if we "guess one" and create some
profile based on current JRE packages. Just read & use the version from
the running JRE.

Fixes eclipse-equinox#461
@laeubi
Copy link
Member

laeubi commented Feb 12, 2024

Please add https://github.com/eclipse-platform/eclipse.platform.releng.aggregator/blob/master/.mvn/jvm.config to this repository as well, it should help with that OOM errors as currently only 1GB of 4G is used by default.

We surely won't have that workaround be forever, is there a ticket for that so I can link the PR against?

If you mean we should stop development once we hit 1gb of memory demands then no there is no such ticket, beside that we don't get an refund if we only use 1/4 of available memory.

@iloveeclipse
Copy link
Member Author

If you mean we should stop development once we hit 1gb of memory demands then no there is no such ticket, beside that we don't get an refund if we only use 1/4 of available memory.

I'm only asking for the root issue here, that requires now this workaround.

@laeubi
Copy link
Member

laeubi commented Feb 12, 2024

If you mean we should stop development once we hit 1gb of memory demands then no there is no such ticket, beside that we don't get an refund if we only use 1/4 of available memory.

I'm only asking for the root issue here, that requires now this workaround.

I have recently analyzed the build and didn't find any unusual memory consumption, so the root cause seem that we are use more features, doing more checks and so on takes more memory so we hit a limit. As long as we are not really hitting the limit (e.g. need more and more and finally getting killed) I won't analyze further. So if 2g works for the next 20years its a bit of waste of dev resources otherwise.

If you are interested, you can run the build local with -Xmx=1g (the current default) and let you produce a heapdump and look for possible optimizations in platform/jdt(I already made some) or tycho/maven (e.g MNG-7592.

@iloveeclipse iloveeclipse merged commit ba4bbf3 into eclipse-equinox:master Feb 12, 2024
9 checks passed
@iloveeclipse iloveeclipse deleted the issue_461 branch February 12, 2024 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UpdateSitePublisher -addJREIU argument doesn't add current JVM
2 participants