ICU-22368 Reduce ~200K langInfo.res size by encode LSR into 32bits int. #2458

FrankYFTang · 2023-05-11T23:13:08Z

Checklist

Required: Issue filed: https://unicode-org.atlassian.net/browse/ICU-22368
Required: The PR title must be prefixed with a JIRA Issue number.
Required: The PR description must include the link to the Jira Issue, for example by completing the URL in the first checklist item
Required: Each commit message must be prefixed with a JIRA Issue number.
Issue accepted (done by Technical Committee after discussion)
Tests included, if applicable
API docs and/or User Guide docs changed or added, if applicable

jira-pull-request-webhook · 2023-05-31T23:34:24Z

Notice: the branch changed across the force-push!

icu4c/source/common/loclikelysubtags.cpp is different
icu4j/main/shared/data/icudata.jar is different
icu4j/main/shared/data/icutzdata.jar is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

FrankYFTang · 2023-06-01T16:29:21Z

This change reduce the size of icudt73l.dat from 32206240 to 31999424 and reduced 206816 bytes

jira-pull-request-webhook · 2023-06-01T23:22:11Z

Notice: the branch changed across the force-push!

icu4j/main/shared/data/icudata.jar is different
icu4j/main/shared/data/icutzdata.jar is different
icu4j/maven-build/maven-icu4j-datafiles/src/main/resources/com/ibm/icu/impl/data/icudt73b/langInfo.res is now changed in the branch

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

FrankYFTang · 2023-06-03T02:32:05Z

Please review. thanks

FrankYFTang · 2023-06-13T18:53:26Z

@pedberg-icu @markusicu could you take a look at this or should I ask someone else?

...to-icu/src/main/java/org/unicode/icu/tool/cldrtoicu/localedistance/LocaleDistanceMapper.java

icu4c/source/common/loclikelysubtags.cpp

icu4j/main/classes/core/src/com/ibm/icu/impl/locale/LSR.java

pedberg-icu · 2023-06-15T03:16:13Z

icu4j/main/classes/core/src/com/ibm/icu/impl/locale/LSR.java

+        // Do not have enough bits to store the all 1000 possible combination of \d{3}
+        // Only support what is needed in resource- "001", "143" and "419".
+        if (region.length() == 3) {
+            assert region.equals("001") || region.equals("143") || region.equals("419");


See my comments about this approach in LocaleDistanceMapper.java. It is too fragile and inflexible. I think LocaleDistanceMapper.java should write an array of supported numeric region codes that this method can use for encoding.

well do. Changing the code now. Stay tune.

Does not seem like this code got updated to use the m49 array?

opps. you are right. need to change it.

pedberg-icu

Needs a better way of handling numeric region codes, see comments (does not even support all currently used codes)

jira-pull-request-webhook · 2023-06-20T17:06:17Z

Notice: the branch changed across the force-push!

icu4c/source/common/loclikelysubtags.cpp is different
icu4c/source/data/misc/langInfo.txt is different
icu4j/main/classes/core/src/com/ibm/icu/impl/locale/LocaleDistance.java is different
icu4j/main/classes/core/src/com/ibm/icu/impl/locale/LSR.java is different
icu4j/main/classes/core/src/com/ibm/icu/impl/locale/XLikelySubtags.java is different
icu4j/main/shared/data/icudata.jar is different
icu4j/main/shared/data/icutzdata.jar is different
icu4j/maven-build/maven-icu4j-datafiles/src/main/resources/com/ibm/icu/impl/data/icudt73b/langInfo.res is different
tools/cldr/cldr-to-icu/src/main/java/org/unicode/icu/tool/cldrtoicu/localedistance/LocaleDistanceMapper.java is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

FrankYFTang · 2023-06-20T17:44:32Z

Peter - I modified the PR to output the M49 codes into the resources from the CLDR to ICU tool and use that during the decoding now.

...to-icu/src/main/java/org/unicode/icu/tool/cldrtoicu/localedistance/LocaleDistanceMapper.java

pedberg-icu · 2023-06-20T21:55:14Z

Peter - I modified the PR to output the M49 codes into the resources from the CLDR to ICU tool and use that during the decoding now.

Oh, I like this much better, thanks! But it looks like LSR.java is still not using the new M49 array...

FrankYFTang · 2023-06-20T23:59:40Z

Needs a better way of handling numeric region codes, see comments (does not even support all currently used codes)

It is not intend to support ALL used codes in everywhere, but only for those encoded in the value of the likelySubtag.xml

FrankYFTang · 2023-06-21T00:00:21Z

Peter - I modified the PR to output the M49 codes into the resources from the CLDR to ICU tool and use that during the decoding now.

Oh, I like this much better, thanks! But it looks like LSR.java is still not using the new M49 array...

yes, sorry, I missed that. Will fix

pedberg-icu

Looks good now, thanks for this!

See unicode-org#2458

jira-pull-request-webhook · 2023-06-21T18:35:52Z

Hooray! The files in the branch are the same across the force-push. 😃

~ Your Friendly Jira-GitHub PR Checker Bot

See unicode-org#2458

FrankYFTang added the incomplete Needs work; do not approve/merge as is. label May 11, 2023

FrankYFTang force-pushed the ICU-22368-second branch from d8327b6 to 80d4070 Compare May 31, 2023 23:34

FrankYFTang changed the title ~~ICU-22368 !!!DO NOT LOOK YET!!! Reduce langInfo.res size by encode LSR into 32bits int.~~ Reduce ~200K langInfo.res size by encode LSR into 32bits int. Jun 1, 2023

FrankYFTang changed the title ~~Reduce ~200K langInfo.res size by encode LSR into 32bits int.~~ ICU-22368 Reduce ~200K langInfo.res size by encode LSR into 32bits int. Jun 1, 2023

FrankYFTang force-pushed the ICU-22368-second branch from 80d4070 to 0f3fb19 Compare June 1, 2023 23:22

FrankYFTang requested review from pedberg-icu and markusicu June 1, 2023 23:37

FrankYFTang added waiting-on-reviewer and removed incomplete Needs work; do not approve/merge as is. labels Jun 3, 2023

FrankYFTang assigned markusicu Jun 9, 2023

pedberg-icu reviewed Jun 15, 2023

View reviewed changes

...to-icu/src/main/java/org/unicode/icu/tool/cldrtoicu/localedistance/LocaleDistanceMapper.java Outdated Show resolved Hide resolved

pedberg-icu reviewed Jun 15, 2023

View reviewed changes

icu4c/source/common/loclikelysubtags.cpp Outdated Show resolved Hide resolved

pedberg-icu reviewed Jun 15, 2023

View reviewed changes

icu4j/main/classes/core/src/com/ibm/icu/impl/locale/LSR.java Outdated Show resolved Hide resolved

pedberg-icu reviewed Jun 15, 2023

View reviewed changes

pedberg-icu requested changes Jun 15, 2023

View reviewed changes

FrankYFTang force-pushed the ICU-22368-second branch from 0f3fb19 to c010bc9 Compare June 20, 2023 17:06

pedberg-icu reviewed Jun 20, 2023

View reviewed changes

...to-icu/src/main/java/org/unicode/icu/tool/cldrtoicu/localedistance/LocaleDistanceMapper.java Show resolved Hide resolved

pedberg-icu reviewed Jun 20, 2023

View reviewed changes

...to-icu/src/main/java/org/unicode/icu/tool/cldrtoicu/localedistance/LocaleDistanceMapper.java Show resolved Hide resolved

pedberg-icu approved these changes Jun 21, 2023

View reviewed changes

ICU-22368 Reduce ~200K langInfo.res size by encode LSR into 32bits int.

4578999

See unicode-org#2458

FrankYFTang force-pushed the ICU-22368-second branch from 2df23e4 to 4578999 Compare June 21, 2023 18:35

FrankYFTang merged commit e83b071 into unicode-org:main Jun 22, 2023
101 checks passed

FrankYFTang deleted the ICU-22368-second branch June 22, 2023 08:18

FrankYFTang mentioned this pull request Aug 8, 2023

ICU-22453 Fix non null terminated buffer issue. #2543

Merged

7 tasks

catamorphism pushed a commit to catamorphism/icu that referenced this pull request Nov 1, 2023

ICU-22368 Reduce ~200K langInfo.res size by encode LSR into 32bits int.

118622e

See unicode-org#2458

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ICU-22368 Reduce ~200K langInfo.res size by encode LSR into 32bits int. #2458

ICU-22368 Reduce ~200K langInfo.res size by encode LSR into 32bits int. #2458

FrankYFTang commented May 11, 2023 •

edited

Loading

jira-pull-request-webhook bot commented May 31, 2023

FrankYFTang commented Jun 1, 2023

jira-pull-request-webhook bot commented Jun 1, 2023

FrankYFTang commented Jun 3, 2023

FrankYFTang commented Jun 13, 2023

pedberg-icu Jun 15, 2023

FrankYFTang Jun 17, 2023

FrankYFTang Jun 20, 2023

pedberg-icu Jun 20, 2023

FrankYFTang Jun 20, 2023

pedberg-icu left a comment

jira-pull-request-webhook bot commented Jun 20, 2023

FrankYFTang commented Jun 20, 2023

pedberg-icu commented Jun 20, 2023

FrankYFTang commented Jun 20, 2023

FrankYFTang commented Jun 21, 2023

pedberg-icu left a comment

jira-pull-request-webhook bot commented Jun 21, 2023

ICU-22368 Reduce ~200K langInfo.res size by encode LSR into 32bits int. #2458

ICU-22368 Reduce ~200K langInfo.res size by encode LSR into 32bits int. #2458

Conversation

FrankYFTang commented May 11, 2023 • edited Loading

Checklist

jira-pull-request-webhook bot commented May 31, 2023

FrankYFTang commented Jun 1, 2023

jira-pull-request-webhook bot commented Jun 1, 2023

FrankYFTang commented Jun 3, 2023

FrankYFTang commented Jun 13, 2023

pedberg-icu Jun 15, 2023

Choose a reason for hiding this comment

FrankYFTang Jun 17, 2023

Choose a reason for hiding this comment

FrankYFTang Jun 20, 2023

Choose a reason for hiding this comment

pedberg-icu Jun 20, 2023

Choose a reason for hiding this comment

FrankYFTang Jun 20, 2023

Choose a reason for hiding this comment

pedberg-icu left a comment

Choose a reason for hiding this comment

jira-pull-request-webhook bot commented Jun 20, 2023

FrankYFTang commented Jun 20, 2023

pedberg-icu commented Jun 20, 2023

FrankYFTang commented Jun 20, 2023

FrankYFTang commented Jun 21, 2023

pedberg-icu left a comment

Choose a reason for hiding this comment

jira-pull-request-webhook bot commented Jun 21, 2023

FrankYFTang commented May 11, 2023 •

edited

Loading