Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(29)

Issue 2952193002: VM: Speed up output of UTF8 for 1-byte strings.

Created:
3 years, 6 months ago by erikcorry
Modified:
3 years, 2 months ago
CC:
reviews_dartlang.org, vm-dev_dartlang.org
Target Ref:
refs/heads/master
Visibility:
Public.

Description

VM: Speed up output of UTF8 for 1-byte strings. For 1-byte strings we don't need the full power of UTF-8 conversion with support for 3 and 4-byte sequences and reassembly of UTF-16 surrogate pairs. This optimization was originally intended to improve the reverse-complement benchmark. It also seems to have a positive effect (ca. 1% on average) on dart2js. After implementing this it doesn't seem worth having a special case in ToCString for ASCII strings. That special case would allocate the string twice for all Latin1 strings, which feels unfortunate. R=vegorov@google.com BUG=

Patch Set 1 #

Patch Set 2 : Clean up a little #

Patch Set 3 : Add NoSafePointScope #

Patch Set 4 : Fix asserts #

Total comments: 22

Patch Set 5 : Slava feedback #

Patch Set 6 : Add test that would have caught bug pointed out by Slava #

Unified diffs Side-by-side diffs Delta from patch set Stats (+159 lines, -66 lines) Patch
M runtime/vm/object.h View 1 2 3 4 6 chunks +22 lines, -0 lines 0 comments Download
M runtime/vm/object.cc View 1 2 3 4 10 chunks +9 lines, -51 lines 0 comments Download
M runtime/vm/raw_object_snapshot.cc View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M runtime/vm/symbols.cc View 1 2 3 4 2 chunks +6 lines, -6 lines 0 comments Download
M runtime/vm/unicode.cc View 1 2 3 4 2 chunks +102 lines, -8 lines 0 comments Download
M runtime/vm/unicode_test.cc View 1 2 3 4 5 1 chunk +19 lines, -0 lines 0 comments Download

Messages

Total messages: 9 (0 generated)
erikcorry
3 years, 6 months ago (2017-06-22 13:45:55 UTC) #1
erikcorry
Please hold off reviewing till Friday.
3 years, 6 months ago (2017-06-22 14:02:53 UTC) #2
erikcorry
The issue right now is that I'm not quite sure why ToCString is OK to ...
3 years, 6 months ago (2017-06-22 14:09:19 UTC) #3
erikcorry
PTAL As I understand it we don't need to be at a safe point to ...
3 years, 5 months ago (2017-07-03 13:22:18 UTC) #4
Vyacheslav Egorov (Google)
https://codereview.chromium.org/2952193002/diff/60001/runtime/vm/unicode.cc File runtime/vm/unicode.cc (right): https://codereview.chromium.org/2952193002/diff/60001/runtime/vm/unicode.cc#newcode134 runtime/vm/unicode.cc:134: if (str.IsOneByteString()) { I think NoSafepointScope should go here, ...
3 years, 5 months ago (2017-07-03 15:49:05 UTC) #5
erikcorry
https://codereview.chromium.org/2952193002/diff/60001/runtime/vm/unicode.cc File runtime/vm/unicode.cc (right): https://codereview.chromium.org/2952193002/diff/60001/runtime/vm/unicode.cc#newcode216 runtime/vm/unicode.cc:216: uintptr_t char_length = Length(src); On 2017/07/03 15:49:05, Vyacheslav Egorov ...
3 years, 5 months ago (2017-07-03 19:40:56 UTC) #6
erikcorry
https://codereview.chromium.org/2952193002/diff/60001/runtime/vm/unicode.cc File runtime/vm/unicode.cc (right): https://codereview.chromium.org/2952193002/diff/60001/runtime/vm/unicode.cc#newcode216 runtime/vm/unicode.cc:216: uintptr_t char_length = Length(src); On 2017/07/03 19:40:56, erikcorry wrote: ...
3 years, 5 months ago (2017-07-03 20:12:15 UTC) #7
erikcorry
https://codereview.chromium.org/2952193002/diff/60001/runtime/vm/unicode.cc File runtime/vm/unicode.cc (right): https://codereview.chromium.org/2952193002/diff/60001/runtime/vm/unicode.cc#newcode134 runtime/vm/unicode.cc:134: if (str.IsOneByteString()) { On 2017/07/03 15:49:05, Vyacheslav Egorov (Google) ...
3 years, 2 months ago (2017-09-25 15:21:52 UTC) #8
erikcorry
3 years, 2 months ago (2017-09-27 14:31:55 UTC) #9

Powered by Google App Engine
This is Rietveld 408576698