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

Intl: ICU 55 to 56 bump #2917

Closed
srl295 opened this issue Sep 16, 2015 · 10 comments
Closed

Intl: ICU 55 to 56 bump #2917

srl295 opened this issue Sep 16, 2015 · 10 comments
Assignees
Labels
i18n-api Issues and PRs related to the i18n implementation. wip Issues and PRs that are still a work in progress.

Comments

@srl295
Copy link
Member

srl295 commented Sep 16, 2015

@srl295 srl295 added the i18n-api Issues and PRs related to the i18n implementation. label Sep 16, 2015
@srl295 srl295 self-assigned this Sep 16, 2015
@srl295
Copy link
Member Author

srl295 commented Sep 25, 2015

ICU 56r1 dropped today. Interested parties can test (noting #3065 which could bite you ) with:

 ./configure --with-icu-source=http://download.icu-project.org/files/icu4c/56rc/icu4c-56_rc-src.zip --with-intl=full-icu --download=all

@rvagg
Copy link
Member

rvagg commented Sep 29, 2015

@srl295 if/when you bump to 56 in the configure could you try and give us a quick summary of improvements that can go in the changelog? Is it simply just "perf"?

@srl295
Copy link
Member Author

srl295 commented Sep 29, 2015

Yes definitely. I will expand on the changes.

There's always fresh data, latest time zone changes and such. But there are
other improvements I will note. No new features.

@srl295
Copy link
Member Author

srl295 commented Oct 7, 2015

Heads up - ICU 56 planned GA today. I'll be submitting a PR soon

@srl295
Copy link
Member Author

srl295 commented Oct 7, 2015

Summarizing from here

  • CLDR 28 5% new data, 8% corrections

Which reminds me, I will need to review the icutrim behavior - some new data categories to trim out.

This would affect string normalization, etc.

  • ICU data size reduced by about 7.2% (1.8MB) via sharing string values across resource bundles.
  • Support for locale key "cf" to specify currency format style, and interaction with NumberFormat values for UNumberFormatStyle: [doc: update to current V8 versions #11787]

I assume this will pass through to Intl but need to test. example would be locale en-us-u-cf-account for negative numbers like (4.56) instead of -4.56

  • Larger UnicodeString object

More on stack, less heap. Should reduce heap usage.

14% to 72% improvements for some cases! Usual benchmark disclaimers apply!

  • Collator: first-time startup time improved 20% due to precalculated unsafe-backward table

My personal contribution to this list.

  • A number of memory leaks and buffer overruns have been fixed based on static code analysis, mostly in data build tools.

And of course,

  • latest time zone data

@srl295 srl295 added the wip Issues and PRs that are still a work in progress. label Oct 7, 2015
@srl295
Copy link
Member Author

srl295 commented Oct 7, 2015

Note that #3065 enables building full-icu, not the default small-icu. Given the timing, I'll fix the remaining 56 issues in this issue instead.

Looks like a false alarm / unclean local build dir. Clean build worked fine.

@srl295
Copy link
Member Author

srl295 commented Oct 8, 2015

Looks like so far, the delta for small-icu and full-icu from ICU 55 to 56 is about 700k on linux x64. May be possible to trim further but i'd probably not (at this point) unless there's a call to.

@srl295
Copy link
Member Author

srl295 commented Oct 8, 2015

perf ICU 56 makes for(var k=0;k<10000;k++) { (123.45) .toLocaleString(); } run about twice as fast as ICU 55. ..

edit and it gets better... var nf = new Intl.NumberFormat(); for (...) { nf.format(123.45); } is more like 3x faster

@srl295
Copy link
Member Author

srl295 commented Oct 8, 2015 via email

@jasnell
Copy link
Member

jasnell commented Oct 8, 2015

@srl295 ... when you get that PR open, please tag it land-on-v4x and mention me in it so I can track it for the 4.2.0 release

srl295 added a commit to srl295/node that referenced this issue Oct 8, 2015
* ICU 56 was just released yesterday. Update to it.
* Notable changes: Unicode 8, CLDR 28, 2-3x number format perf,
  20% improvement in Collator startup
  * more at http://site.icu-project.org/download/56 or in nodejs#2917

Also:

* cleanup out/**/*.d and deps/icu  on "make clean"

Fixes: nodejs#2917
@srl295 srl295 closed this as completed in 01908d0 Oct 8, 2015
srl295 added a commit that referenced this issue Oct 9, 2015
* ICU 56 was just released yesterday. Update to it.
* Notable changes: Unicode 8, CLDR 28, 2-3x number format perf,
  20% improvement in Collator startup
  * more at http://site.icu-project.org/download/56 or in #2917

Also:

* cleanup out/**/*.d and deps/icu  on "make clean"
* cleanup deps/icu on "vcbuild clean"

When building from an non-clean directory, it's important to
run `make clean` or `vcbuild clean` to remove the existing
ICU 55 from the deps path before building.

Fixes: #2917
PR-URL: #3281
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
i18n-api Issues and PRs related to the i18n implementation. wip Issues and PRs that are still a work in progress.
Projects
None yet
Development

No branches or pull requests

3 participants