Summary: | [JSC] String.fromCharCode's slow path always generates 16bit string | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Yusuke Suzuki <ysuzuki> | ||||||
Component: | New Bugs | Assignee: | Yusuke Suzuki <ysuzuki> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | darin, ews-watchlist, keith_miller, mark.lam, msaboff, saam, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Yusuke Suzuki
2019-02-08 18:24:11 PST
Created attachment 361577 [details]
Patch
Comment on attachment 361577 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=361577&action=review r=me with small fix. > Source/JavaScriptCore/ChangeLog:10 > + and even worse, taints ropes 16bit if 16bit string is included in the given rope. We find that acorn-wtb Typo: We find => We found. Or if you want to take more credit: I found. > Source/JavaScriptCore/ChangeLog:12 > + 16bit string. However, only few strings are actually 16bit strings. This patch attempts to make 8bit string Typo: only few => only a few > Source/JavaScriptCore/runtime/StringConstructor.cpp:91 > + if (UNLIKELY(character & 0xFF00)) { Can we use or create a named WTF function for this? Comment on attachment 361577 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=361577&action=review >> Source/JavaScriptCore/runtime/StringConstructor.cpp:91 >> + if (UNLIKELY(character & 0xFF00)) { > > Can we use or create a named WTF function for this? Yeah, I added `is8BitCharacter(ch)`, since ASCIICType::isASCII(ch) is different from this definition. (It does not allow the upper most bit is set, since it is precisely out of ASCII range). Comment on attachment 361577 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=361577&action=review >> Source/JavaScriptCore/ChangeLog:10 >> + and even worse, taints ropes 16bit if 16bit string is included in the given rope. We find that acorn-wtb > > Typo: We find => We found. Or if you want to take more credit: I found. Fixed. >> Source/JavaScriptCore/ChangeLog:12 >> + 16bit string. However, only few strings are actually 16bit strings. This patch attempts to make 8bit string > > Typo: only few => only a few Fixed. (In reply to Yusuke Suzuki from comment #3) > Comment on attachment 361577 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=361577&action=review > > >> Source/JavaScriptCore/runtime/StringConstructor.cpp:91 > >> + if (UNLIKELY(character & 0xFF00)) { > > > > Can we use or create a named WTF function for this? > > Yeah, I added `is8BitCharacter(ch)`, since ASCIICType::isASCII(ch) is > different from this definition. (It does not allow the upper most bit is > set, since it is precisely out of ASCII range). I think we should actually call it isLatin1. IIUC, that's the most precise term. Comment on attachment 361577 [details] Patch Attachment 361577 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/11087027 New failing tests: imported/w3c/web-platform-tests/webrtc/simplecall.https.html Created attachment 361591 [details]
Archive of layout-test-results from ews121 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews121 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.6
Committed r241233: <https://trac.webkit.org/changeset/241233> (In reply to Keith Miller from comment #5) > I think we should actually call it isLatin1. IIUC, that's the most precise > term. There are isLatin1 functions in Lexer.cpp already. This patch adds a new one. Could we consolidate? (In reply to Darin Adler from comment #10) > (In reply to Keith Miller from comment #5) > > I think we should actually call it isLatin1. IIUC, that's the most precise > > term. > > There are isLatin1 functions in Lexer.cpp already. This patch adds a new > one. Could we consolidate? Oh, nice! I'll land it in a follow-up patch. Committed r241246: <https://trac.webkit.org/changeset/241246> |