Suggest a rework for DictionaryData in java port. #523

New Issue

claunia · 2026-01-29T20:45:10Z

claunia commented

2026-01-29 20:45:10 +00:00

Originally created by @XenoAmess on GitHub (Oct 21, 2024).

just, don't make a 123KB class if not have to. that is not a good way.
a better practice would be:

remove the reflection in Dictionary.
rework DictionaryData to load from resources file.
move the load part from static part to method invocation part, and make a 2-step-sync to the loading part.

would be glad to impl it if you would arrange time for the code review.

Originally created by @XenoAmess on GitHub (Oct 21, 2024). just, don't make a 123KB class if not have to. that is not a good way. a better practice would be: 1. remove the reflection in Dictionary. 2. rework DictionaryData to load from resources file. 3. move the load part from static part to method invocation part, and make a 2-step-sync to the loading part. would be glad to impl it if you would arrange time for the code review.

claunia commented

2026-01-29 20:45:10 +00:00

@eustas commented on GitHub (Nov 19, 2024):

This design has a long history and meets some non-obvious requirements:

make sure it is zero-cost even at class tree is population time
do not rely on resource loading
allow dropping dictionary at all

With current implementation, reflection is not triggered if embedder has already invoked Dictionary.setData; in such case provided data could have been obtained by different means, e.g. loaded from resources.

If the main problem is class size, then you can compile/link with alternative DictionaryData implementation that loads from resources.

Please provide more context on the problem you are facing, and we could think how to improve the codebase.

@eustas commented on GitHub (Nov 19, 2024): This design has a long history and meets some non-obvious requirements: - make sure it is zero-cost even at class tree is population time - do not rely on resource loading - allow dropping dictionary at all With current implementation, reflection is not triggered if embedder has already invoked `Dictionary.setData`; in such case provided data could have been obtained by different means, e.g. loaded from resources. If the main problem is class size, then you can compile/link with alternative DictionaryData implementation that loads from resources. Please provide more context on the problem you are facing, and we could think how to improve the codebase.

claunia commented

2026-01-29 20:45:11 +00:00

@XenoAmess commented on GitHub (Nov 19, 2024):

This design has a long history and meets some non-obvious requirements:

make sure it is zero-cost even at class tree is population time

do not rely on resource loading

allow dropping dictionary at all

With current implementation, reflection is not triggered if embedder has already invoked Dictionary.setData; in such case provided data could have been obtained by different means, e.g. loaded from resources.

If the main problem is class size, then you can compile/link with alternative DictionaryData implementation that loads from resources.

Please provide more context on the problem you are facing, and we could think how to improve the codebase.

I'm using graalvm native image, so of course as small class as possible and as few reflections as possible means something to me....

But the real reason if I remember it correctly why I give up with brotli last month is I cannot uncompress some brotli file (generated by brotli cli) using the java client...

now I'm using zip as alternative.

@XenoAmess commented on GitHub (Nov 19, 2024): > This design has a long history and meets some non-obvious requirements: > > * make sure it is zero-cost even at class tree is population time > * do not rely on resource loading > * allow dropping dictionary at all > > With current implementation, reflection is not triggered if embedder has already invoked `Dictionary.setData`; in such case provided data could have been obtained by different means, e.g. loaded from resources. > > If the main problem is class size, then you can compile/link with alternative DictionaryData implementation that loads from resources. > > Please provide more context on the problem you are facing, and we could think how to improve the codebase. I'm using graalvm native image, so of course as small class as possible and as few reflections as possible means something to me.... But the real reason if I remember it correctly why I give up with brotli last month is I cannot uncompress some brotli file (generated by brotli cli) using the java client... now I'm using zip as alternative.

claunia commented

2026-01-29 20:45:11 +00:00

@eustas commented on GitHub (Nov 20, 2024):

Got it. I'll prepare helper for reflection-less load-from-resources soon; the biggest question is - what build system (or binary repo) you use?

@eustas commented on GitHub (Nov 20, 2024): Got it. I'll prepare helper for reflection-less load-from-resources soon; the biggest question is - what build system (or binary repo) you use?

claunia referenced this issue

2026-01-29 20:52:47 +00:00

[PR #523] [MERGED] Better compression #1165

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: starred/brotli#523