Closed
Bug 1431025
Opened 7 years ago
Closed 7 years ago
Replace nsBidiUtils::HasRTLChars() implementation with encoding_rs
Categories
(Core :: Internationalization, enhancement)
Core
Internationalization
Tracking
()
RESOLVED
FIXED
mozilla60
Tracking | Status | |
---|---|---|
firefox60 | --- | fixed |
People
(Reporter: hsivonen, Assigned: hsivonen)
References
Details
Attachments
(1 file)
I have a Rust SIMD implementation of UTF-16 bidiness check that on a Haswell i7 shows the following improvement compared to a Rust ALU implementation that I believe (but haven't verified) to be faster than our current C++ ALU code:
bench_mem_is_utf16_bidi_de_1000 261,488 (3824 MB/s) 38,708 (25834 MB/s) -222,780 -85.20%
bench_mem_is_utf16_bidi_ja_1000 524,420 (1906 MB/s) 125,482 (7969 MB/s) -398,938 -76.07%
bench_mem_is_utf16_bidi_ru_1000 292,927 (3413 MB/s) 64,627 (15473 MB/s) -228,300 -77.94%
bench_mem_is_utf16_bidi_th_1000 503,164 (1987 MB/s) 89,577 (11163 MB/s) -413,587 -82.20%
We should use the fast code in Gecko.
(I don't know why Japanese shows a lesser improvement than Thai. Based on the structure of the code, they should benefit equally.)
Comment hidden (mozreview-request) |
Assignee | ||
Comment 2•7 years ago
|
||
For non-PGO, this is indeed even a bigger win compared to our C++ than compared to ALU Rust:
https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=1d611c4897eb8b6a1115ef71b087bd93c9027620&newProject=try&newRevision=a597f0b9d49b0c665244a3caf3dbdc52ed33efeb&framework=6
Compared to PGO C++, it's not as big a win as compared to non-PGO ALU Rust, but it's still a very big win.
Assignee | ||
Comment 3•7 years ago
|
||
(In reply to Henri Sivonen (:hsivonen) from comment #2)
> Compared to PGO C++, it's not as big a win as compared to non-PGO ALU Rust,
> but it's still a very big win.
For scripts below the main bidi block. For the script above the main bidi block, it looks like C++ PGO is a pessimization, so the win compared to PGO is greater than the win compared to non-PGO.
Assignee | ||
Updated•7 years ago
|
Attachment #8943578 -
Flags: review?(jfkthame)
Comment 4•7 years ago
|
||
mozreview-review |
Comment on attachment 8943578 [details]
Bug 1431025 - Use encoding_rs::mem::is_utf16_bidi() as the implementation of HasRTLChars().
https://reviewboard.mozilla.org/r/213924/#review221208
::: intl/unicharutil/util/nsBidiUtils.h:162
(Diff revision 1)
> /**
> - * Give a 16-bit (UTF-16) text buffer and length
> + * Give a 16-bit (UTF-16) text buffer
> * @return true if the string contains right-to-left characters
> */
> - bool HasRTLChars(const char16_t* aText, uint32_t aLength);
> -
> + inline bool HasRTLChars(mozilla::Span<const char16_t> aBuffer) {
> + // Span ensures we never pass a nullptr to Rust--even if the
trailing space
Attachment #8943578 -
Flags: review?(jfkthame) → review+
Comment hidden (mozreview-request) |
Assignee | ||
Comment 6•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8943578 [details]
Bug 1431025 - Use encoding_rs::mem::is_utf16_bidi() as the implementation of HasRTLChars().
https://reviewboard.mozilla.org/r/213924/#review221208
> trailing space
Fixed. Thanks.
Pushed by hsivonen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9a92be6b431a
Use encoding_rs::mem::is_utf16_bidi() as the implementation of HasRTLChars(). r=jfkthame
Comment 8•7 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox60:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
Updated•6 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•