Created attachment 360591[details]
Archive of layout-test-results from ews102 for mac-highsierra
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102 Port: mac-highsierra Platform: Mac OS X 10.13.6
Created attachment 360592[details]
Archive of layout-test-results from ews106 for mac-highsierra-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106 Port: mac-highsierra-wk2 Platform: Mac OS X 10.13.6
Created attachment 360595[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
Created attachment 360599[details]
Archive of layout-test-results from ews114 for mac-highsierra
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114 Port: mac-highsierra Platform: Mac OS X 10.13.6
Created attachment 360959[details]
Archive of layout-test-results from ews101 for mac-highsierra
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101 Port: mac-highsierra Platform: Mac OS X 10.13.6
Created attachment 360960[details]
Archive of layout-test-results from ews104 for mac-highsierra-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104 Port: mac-highsierra-wk2 Platform: Mac OS X 10.13.6
Created attachment 360962[details]
Archive of layout-test-results from ews114 for mac-highsierra
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114 Port: mac-highsierra Platform: Mac OS X 10.13.6
Created attachment 360965[details]
Archive of layout-test-results from ews206 for win-future
The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews206 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Created attachment 360966[details]
Archive of layout-test-results from ews123 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.6
Created attachment 361279[details]
Archive of layout-test-results from ews124 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.6
Comment on attachment 361606[details]
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=361606&action=review
I think this is ready for review now. Basically all tests passing everything compiling on all platforms.
One small nit right at the moment: Not sure why there are tests failing on ios-sim buildbot (still running, so maybe it's a glitch); did not see those tests fail with the almost identical previous version of the same patch (was all green).
I did not touch the serialization done by the function named makeString in Wasm nor call sites like JSC::BuiltinNames::getPublicName; there are some places in both categories that allocate unnecessary String temporaries and that could be using StringView or StringConcatenateNumbers instead, but things got a little too complicated for me to understand how to make safe fixes there without deeper thinking and more care. I think we should probably rename the makeString function in WasmOps.h to avoid confusion with the makeString in WTF since they are different and it’s easy to be confused about which one a given call site is calling.
Note however, that sometimes what looks like an unnecessary String temporary is just reference counting churn, making a String since makeString doesn’t work with StringImpl directly. That’s much less expensive than allocating/destroying a StringImpl, which is the bigger thing we are fixing in most cases here in this patch.
The only significant challenge in getting this patch correct was that for floating point numbers, the makeString default is ECMAScript-style shortest form, and the String::number and StringBuilder::appendNumber default is 6 digits, fixed precision, strip trailing zeroes. For many cases, the two approaches are basically equivalent and it’s nice to move to the shortest form as a more elegant solution to the same problem, but a big exception is when it’s a float rather than a double, because we don’t currently have a float shortest form implementation. So for those cases, I had to be careful to preserve the use of fixed precision. Would be nice to fix that eventually so we can reduce the unnecessary use of fixed precision, since shortest is almost always better. In fact, if we fixed that I would also want to change the String::number and StringBuilder::appendNumber defaults as well. So, for now, any place where String::number is replaced by the use of makeString, if it’s an integer or if it’s a double and shortest form is equivalent/OK we can do it in the simplest form, but if it’s a float then we almost always have to use FormattedNumber::fixedPrecision to preserve behavior for now.
> Source/JavaScriptCore/runtime/NumberPrototype.cpp:468
> - NumberToStringBuffer buffer;
> - return JSValue::encode(jsString(exec, String(numberToFixedWidthString(x, decimalPlaces, buffer))));
> + return JSValue::encode(jsString(exec, String::numberToStringFixedWidth(x, decimalPlaces)));
This does add one function call: I’ll need to double check to be sure this doesn’t slow things down, but it’s unlikely.
> Source/JavaScriptCore/runtime/NumberPrototype.cpp:504
> - NumberToStringBuffer buffer;
> - return JSValue::encode(jsString(exec, String(numberToFixedPrecisionString(x, significantFigures, buffer))));
> + return JSValue::encode(jsString(exec, String::number(x, significantFigures, KeepTrailingZeros)));
Ditto.
Created attachment 361609[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
2019-01-30 08:37 PST, Darin Adler
2019-01-30 09:03 PST, Darin Adler
2019-01-30 09:05 PST, Darin Adler
2019-01-30 09:17 PST, Darin Adler
2019-01-30 10:29 PST, EWS Watchlist
2019-01-30 10:29 PST, EWS Watchlist
2019-01-30 10:47 PST, EWS Watchlist
2019-01-30 11:08 PST, EWS Watchlist
2019-02-01 19:19 PST, Darin Adler
2019-02-01 19:42 PST, Darin Adler
2019-02-01 20:14 PST, Darin Adler
2019-02-01 21:26 PST, EWS Watchlist
2019-02-01 21:34 PST, EWS Watchlist
2019-02-01 22:07 PST, EWS Watchlist
2019-02-01 22:33 PST, EWS Watchlist
2019-02-01 23:22 PST, EWS Watchlist
2019-02-03 09:35 PST, Darin Adler
2019-02-05 21:26 PST, Darin Adler
2019-02-05 23:15 PST, EWS Watchlist
2019-02-08 18:55 PST, Darin Adler
2019-02-09 08:13 PST, Darin Adler
ews-watchlist: commit-queue-
2019-02-09 10:22 PST, EWS Watchlist