JSC is gonna be sweeeeeet.
Created attachment 161108 [details] Patch
The EFL EWS is gonna fail, there is nothing I can do about it with the patch. The bot does not regenerate the bindings and it fails to build because the old generated files reference UString.
Please test SunSpider before landing. It's heavy in the string library functions.
Thanks for the review, I was not expecting one tonight! :) (In reply to comment #3) > Please test SunSpider before landing. It's heavy in the string library functions. Will do.
Comment on attachment 161108 [details] Patch Attachment 161108 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/13683016
Comment on attachment 161108 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=161108&action=review > Source/WebCore/bindings/js/JSIntentConstructor.cpp:68 > + const String type(MAYBE_MISSING_PARAMETER(exec, 1, DefaultIsUndefined).isEmpty() ? String() : MAYBE_MISSING_PARAMETER(exec, 1, DefaultIsUndefined).toWTFString(exec))); It looks unneeded ")" is added at the end of line. When I remove ")", EFL build is fine.
> > Source/WebCore/bindings/js/JSIntentConstructor.cpp:68 > > + const String type(MAYBE_MISSING_PARAMETER(exec, 1, DefaultIsUndefined).isEmpty() ? String() : MAYBE_MISSING_PARAMETER(exec, 1, DefaultIsUndefined).toWTFString(exec))); > > It looks unneeded ")" is added at the end of line. When I remove ")", EFL build is fine. Thanks for looking! I'll make an update tomorrow.
Created attachment 161396 [details] Patch
Changes since last patch: -Added getCharactersWithUpconvert<>() because the behavior of UString::getCharacters<>() was slightly different and we need the other behavior for StringPrototype. -Added back WebCore::jsString, renamed it to jsStringWithCache so it is easier to differentiate when to use which function. I haven't measured any difference on Sunspider with this version. Gavin gave me some tricks to perform more perf testing, I'll try that before landing. The WebCore binary got even smaller with the changes and I have no clue why. The EFL bot is covered, the GTK bot will be.
Totally awesome stuff :) (In reply to comment #9) > The WebCore binary got even smaller with the changes and I have no clue why. Perhaps it is the reduced number of entries in the PLT that contributes to the reduction in size.
Comment on attachment 161396 [details] Patch Attachment 161396 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/13694551
Comment on attachment 161396 [details] Patch Attachment 161396 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/13681556
Comment on attachment 161396 [details] Patch Attachment 161396 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/13683609
I'm looking into the GTK build failures and will hopefully produce a fix soon.
Created attachment 161411 [details] Patch for GTK build fix The symbols.filter file just needs an update for the jsStringWithCacheSlowCase symbol. With that the build should be fixed.
> The symbols.filter file just needs an update for the jsStringWithCacheSlowCase symbol. With that the build should be fixed. Aw snap. Thanks for the patch. I'll make an update.
Created attachment 161423 [details] Patch
> Totally awesome stuff :) Glad you like it :) > Perhaps it is the reduced number of entries in the PLT that contributes to the reduction in size. Maybe. I don't think I removed symbols between the two patches but I could have forgot.
Comment on attachment 161423 [details] Patch Attachment 161423 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/13693649
Comment on attachment 161423 [details] Patch Attachment 161423 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/13694610
Created attachment 161492 [details] Patch
Comment on attachment 161492 [details] Patch Attachment 161492 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/13691771
Comment on attachment 161492 [details] Patch Attachment 161492 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/13695756
Comment on attachment 161492 [details] Patch r=me
Benjamin, May I have 10 minutes just to try the patch locally here on EFL?
(In reply to comment #25) > Benjamin, > > May I have 10 minutes just to try the patch locally here on EFL? It is indeed failing, I tried a clean build. Need a couple more minutes to fix.
This patch is good to go on EFL after the change bellow (but looks unrelated to the error reported by the bot). Please apply the change and ignore any further EFL EWS errors on this patch: diff --git a/Source/WebCore/bindings/js/Dictionary.cpp b/Source/WebCore/bindings/js/Dictionary.cpp index a16b5b9..e126b95 100644 --- a/Source/WebCore/bindings/js/Dictionary.cpp +++ b/Source/WebCore/bindings/js/Dictionary.cpp @@ -90,7 +90,7 @@ bool Dictionary::getOwnPropertyNames(WTF::Vector<String>& names) const PropertyNameArray propertyNames(exec); JSObject::getOwnPropertyNames(object, exec, propertyNames, ExcludeDontEnumProperties); for (PropertyNameArray::const_iterator it = propertyNames.begin(); it != propertyNames.end(); it++) { - String stringKey = ustringToString(it->ustring()); + const String& stringKey = it->ustring(); if (!stringKey.isEmpty()) names.append(stringKey); }
Committed r127191: <http://trac.webkit.org/changeset/127191>