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

[memory] intern ConfigurationElement.propertiesAndValue #536

Conversation

jukzi
Copy link
Contributor

@jukzi jukzi commented Feb 21, 2024

o.e.core.internal.registry.ConfigurationElement.propertiesAndValue is likely to contain duplicate Strings like "true"

@jukzi
Copy link
Contributor Author

jukzi commented Feb 21, 2024

I would like to get rid of this 4MB overhead:
image

@@ -299,7 +299,7 @@ private String[] readPropertiesAndValue(DataInputStream inputStream) throws IOEx
return RegistryObjectManager.EMPTY_STRING_ARRAY;
String[] properties = new String[numberOfProperties];
for (int i = 0; i < numberOfProperties; i++) {
properties[i] = readStringOrNull(inputStream);
properties[i] = readStringOrNull(inputStream).intern();
Copy link
Member

Choose a reason for hiding this comment

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

The method name suggest it could be null, maybe better intern in the method itself?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks for the hint - in which method?

Copy link
Member

Choose a reason for hiding this comment

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

thanks for the hint - in which method?

readStringOrNull

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if there are two such methods:

image

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if there are two such methods:

image

Well the one that this TabelReader calls then...

@jukzi jukzi force-pushed the intern_ConfigurationElement.propertiesAndValue branch from f1d6c6e to dc03374 Compare February 21, 2024 13:53
Copy link

github-actions bot commented Feb 21, 2024

Test Results

   28 files  ±0     28 suites  ±0   11m 41s ⏱️ +7s
2 170 tests ±0  2 124 ✅ ±0  46 💤 ±0  0 ❌ ±0 
2 242 runs  ±0  2 196 ✅ ±0  46 💤 ±0  0 ❌ ±0 

Results for commit 051634e. ± Comparison against base commit 817ba32.

♻️ This comment has been updated with latest results.

@jukzi jukzi force-pushed the intern_ConfigurationElement.propertiesAndValue branch from dc03374 to 98fd256 Compare February 22, 2024 11:20
o.e.core.internal.registry.ConfigurationElement.propertiesAndValue
is likely to contain duplicate Strings like "true"
@jukzi jukzi force-pushed the intern_ConfigurationElement.propertiesAndValue branch from 98fd256 to 52bce90 Compare March 5, 2024 07:12
@jukzi jukzi merged commit 2d152e2 into eclipse-equinox:master Mar 5, 2024
27 of 28 checks passed
@jukzi jukzi deleted the intern_ConfigurationElement.propertiesAndValue branch March 5, 2024 11:58
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.

4 participants