User-Agent: Mozilla/4.0 (compatible; MSIE 7.0; Windows NT 5.1; InfoPath.1; .NET CLR 1.1.4322; .NET CLR 2.0.50727; .NET CLR 3.0.04506.648; .NET CLR 3.5.21022) Build Identifier: The TT VM method String::testSingleByte is the hot spot of Sunspider benchmark string-validate-input. The method involves a loop that scans a given string looking for a byte whose most significant bit is set. By using the SSE2 version of this loop (actually simple to implement in C++ with MSVC intrinsic), the performance of TT on this benhmark almost doubles. The SSE2 code is guarded by a proper check to kick in only when the length of the string is larger than a threshold. The implementation has no negative impact on other cases involving short strings. Reproducible: Always Steps to Reproduce: 1. 2. 3.
Created attachment 310078 [details] [diff] [review] sse2 version of method String::testSingleByte gc->core()->sse_opt should be accessable in String::testSingleByte. This patch does not build with Release since gc->core() is not avaialble.
You want to pass an AvmCore * down through the callers of testSingleByte, I would refactor the String ctor's to take a core instead and they can get the GC from that. Also the intrinsic includes should be in ifdef for IA32 and MSVC.
we should be able to use these intrinsics on macintel as well, right?
Yes, in fact in avmplus.h we already have: #if defined(_MAC) && (defined (AVMPLUS_IA32) || defined(AVMPLUS_AMD64)) #include <emmintrin.h> #endif
Created attachment 310286 [details] [diff] [review] Refactoring the code according to Tommy Reilly 's comments - Refactor the String constructor to take a core instead of gc - Pass an AvmCore * down through the callers of testSingleByte - Guard the SSE2 code and the header files in ifdef
Attachment #310078 - Attachment is obsolete: true
Created attachment 310288 [details] [diff] [review] Here is the patch
Attachment #310286 - Attachment is obsolete: true
Getting there... #if defined (AVMPLUS_IA32) bufa = (utf8_t *)((uint32_t) buf & 0xFFFFFFF0)+16; #else bufa = (utf8_t *)((uint64_t) buf & 0xFFFFFFF0)+16; #endif Could be: bufa = (utf8_t *)((uintptr_t) buf & 0xFFFFFFF0)+16; I'm not sure about the mac intrinsic header names, have this been tested on OS X? We lost an important String ctor inline, I guess that's because we need to call AvmCore::GetGC(). Oh well, that's fine I guess. Actually setBuff isn't an inline so you could pass AvmCore* to it and leave the String ctor inlined.
Created attachment 310527 [details] [diff] [review] refactor testSingleByte Refactored testSingleByte() using uintptr_t instead of uint32_t/uint64_t. Tested on OS X and there is no problem with the intrinsic header names. About the inline of String ctor, since the String ctor is only called within StringObject.cpp, adding keyword inline to the ctor should work. Actually, just checked the asm in VS, the String ctor is not inlined anyway, neither as the orignal(the ctor in .h), nor using inline keyword in .cpp.
Attachment #310288 - Attachment is obsolete: true
Can we generate a diff against tip with hg diff? Thats how we usually do it. Thanks!
Created attachment 312081 [details] [diff] [review] Diff against tip Just generated a diff against tip using "hg diff -r tip". Let me know if it is not what you want. I am still new to hg... Thanks.
Attachment #310527 - Attachment is obsolete: true
Sure you did a pull/update before doing hg diff? treilly:tt-string treilly$ curl https://bugzilla.mozilla.org/attachment.cgi?id=312081 | hg import - % Total % Received % Xferd Average Speed Time Time Time Current Dload Upload Total Spent Left Speed 100 5652 100 5652 0 0 26654 0 --:--:-- --:--:-- --:--:-- 1839k applying patch from stdin patching file core/StringObject.cpp Hunk #1 FAILED at 37 Hunk #2 FAILED at 72 Hunk #3 FAILED at 96 Hunk #4 FAILED at 109 Hunk #5 FAILED at 121 Hunk #6 FAILED at 220 Hunk #7 FAILED at 253 7 out of 7 hunks FAILED -- saving rejects to file core/StringObject.cpp.rej patching file core/StringObject.h Hunk #1 FAILED at 154 Hunk #2 FAILED at 163 Hunk #3 FAILED at 348 3 out of 3 hunks FAILED -- saving rejects to file core/StringObject.h.rej patching file core/avmplus.h Hunk #1 FAILED at 97 1 out of 1 hunk FAILED -- saving rejects to file core/avmplus.h.rej abort: patch failed to apply
I did execute pull/update before the diff. Did I miss anything? Thanks! >hg tip changeset: 322:db21d04bc48e tag: tip user: Edwin Smith <firstname.lastname@example.org> date: Tue Mar 18 10:14:38 2008 -0400 summary: MinBuild fix >hg pull pulling from http://hg.mozilla.org/tamarin-tracing/ searching for changes no changes found >hg update 0 files updated, 0 files merged, 0 files removed, 0 files unresolved >hg diff -r tip > patch.txt
Never mind, I was getting tripped up by the windows style line endings (import diff on mac). Got it now, I'll push it soon.
pushed in rev 323
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.