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

use xtext string interning for low memory devices #1859

Merged
merged 1 commit into from May 12, 2024

Conversation

joerg1985
Copy link
Contributor

This PR will enable xtext string interning for low memory devices to get rid of a lot Strings.
For my system this will reduce the number of strings inside a memory dump by ~50%.

https://github.com/eclipse/xtext/blob/main/org.eclipse.xtext/src/org/eclipse/xtext/naming/QualifiedName.java#L130-L137

@mstormi
Copy link
Contributor

mstormi commented Mar 26, 2024

Thanks for raising.
Is there any reason why you applied this on very low mem machines only ?
Plus, looking at the linked issue, any reason not to also deploy -XX:+UseG1GC -XX:+UseStringDeduplication as well ?

@joerg1985
Copy link
Contributor Author

This change could be applied to all memory sizes, but this change is using less memory by using more cpu. When alot of memory is available there is no need to have this enabled.
I am not familar with the Azul Zulu JVM and using GC1 (with this flag) might break something.

@mstormi
Copy link
Contributor

mstormi commented Mar 26, 2024

cpu is absolutely not an issue with OH, mem almost always is. Note on openHABian there's usually more programs running to use RAM (e.g. nodejs), and there's swapping/paging to be avoided (even more so if no ZRAM) with all sizes.

WRT Zulu, all flags must work with all JDKs or at least get ignored by those that don't support an option.
Then again this isn't Zulu specific, just JVM 17.

Please check JVM version before adding.

EDIT
I suggest you change the PR to do -Dxtext.qn.interning=true -XX:-TieredCompilation -XX:G1PeriodicGCInterval=60000 -XX:G1PeriodicGCSystemLoadThreshold=4 -XX:MinHeapFreeRatio=4 -XX:MaxHeapFreeRatio=12 on ALL mem sizes

plus -Xss1024k on lowmem boxes

Copy link
Contributor

@mstormi mstormi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please test with the full range of Java opts I gave in my (edited) comment
and change this PR to include them all, if applicable for all mem sizes.

@mstormi
Copy link
Contributor

mstormi commented Apr 9, 2024

does that thumb-up mean you will be testing & updating the PR ?

@joerg1985
Copy link
Contributor Author

Yes, but testing might take some time to see if it is working and not just starting with these flags.

@joerg1985
Copy link
Contributor Author

When all these flags set i have issues calling a graph with one year history, the 4 cores of my raspi 3a stuck at 100% and 60°C for ~4 minutes. I will try some other settings to fix this.

@mstormi
Copy link
Contributor

mstormi commented Apr 11, 2024

When all these flags set i have issues calling a graph with one year history, the 4 cores of my raspi 3a stuck at 100% and 60°C for ~4 minutes. I will try some other settings to fix this.

Ok please remember to double check i.e. if if is a) reproduceable and b) does not occur without those option.
Processing a full year's data is a lot on a 1GB box I wouldn't be sure if that works on current openHABian.

Signed-off-by: Jörg Sautter <joerg.sautter@gmx.net>
@joerg1985
Copy link
Contributor Author

@mstormi i have updated the PR, some flags had no (-XX:G1PeriodicGCInterval=60000 -XX:G1PeriodicGCSystemLoadThreshold=4) or negative (-XX:MinHeapFreeRatio=4 -XX:MaxHeapFreeRatio=12) impact so i dropped them.

The negative impact was reproducible and resulted probably from the overhead by releasing and acquiring the memory to aggressive.

@mstormi
Copy link
Contributor

mstormi commented May 12, 2024

Thanks. Did you exclusively test on a 1GB box i.e. RPi 3A ?

@mstormi mstormi merged commit 4c3cd93 into openhab:main May 12, 2024
5 of 6 checks passed
@joerg1985
Copy link
Contributor Author

Thanks. Did you exclusively test on a 1GB box i.e. RPi 3A ?

Yes, but it is a 512 MB box with zram (zstd, 280%).

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.

None yet

2 participants