Bug 70937 - Increase StringImpl Flag Bits for 8 bit Strings
Summary: Increase StringImpl Flag Bits for 8 bit Strings
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Michael Saboff
URL:
Keywords:
Depends on:
Blocks: 66161
  Show dependency treegraph
 
Reported: 2011-10-26 09:43 PDT by Michael Saboff
Modified: 2011-10-27 17:53 PDT (History)
9 users (show)

See Also:


Attachments
Proposed patch (5.56 KB, patch)
2011-10-26 09:54 PDT, Michael Saboff
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Saboff 2011-10-26 09:43:42 PDT
This change is in preparation for adding 8-bit string support.  Two flag bits are needed in StringImpl for 8 bit string support.
Comment 1 Michael Saboff 2011-10-26 09:54:44 PDT
Created attachment 112551 [details]
Proposed patch
Comment 2 Darin Adler 2011-10-26 10:06:06 PDT
Comment on attachment 112551 [details]
Proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=112551&action=review

> Source/JavaScriptCore/ChangeLog:9
> +        Increased the number of bits used for flags in StringImpl
> +        from 6 to 8 bits.  Updated hash methods accordingly.
> +        Changed hash value masking from the low bits to the high
> +        bits.

This should say why. “This frees up 2 bits that we will be using for 8-bit string support.”
Comment 3 Michael Saboff 2011-10-26 10:12:36 PDT
Committed r98495: <http://trac.webkit.org/changeset/98495>
Comment 4 Ryosuke Niwa 2011-10-26 21:32:32 PDT
It appears that the following tests started failing after this patch:
http/tests/inspector/search/search-in-resources.html
inspector/storage-panel-dom-storage.html

It's totally unclear why this patch can possibly cause these tests to fail.
Comment 5 Ryosuke Niwa 2011-10-26 21:32:57 PDT
http://build.webkit.org/results/SnowLeopard%20Intel%20Release%20(Tests)/r98530%20(34228)/results.html

FYI, I've manually bisected on my SL machine using Apple's Mac port.
Comment 6 Ryosuke Niwa 2011-10-26 21:36:21 PDT
I'm sorry to tell you this 11 hours after your patch was landed but svn server was down for the whole day, bots were unstable, etc... and I could not figure it out earlier :(
Comment 7 Csaba Osztrogonác 2011-10-27 01:45:10 PDT
I can confirm that it broke fast/dom/prototype-inheritance-2.html and inspector/storage-panel-dom-storage.html on the Qt platform too. (with manual bisecting because of svn break)
Comment 8 Csaba Osztrogonác 2011-10-27 02:18:34 PDT
(In reply to comment #7)
> I can confirm that it broke fast/dom/prototype-inheritance-2.html and inspector/storage-panel-dom-storage.html on the Qt platform too. (with manual bisecting because of svn break)

Only fast/dom/prototype-inheritance-2.html, other test failed because of https://bugs.webkit.org/show_bug.cgi?id=70985
Comment 9 Zoltan Herczeg 2011-10-27 05:35:05 PDT
My impression is that the changed hash codes changed the order of items in some tables. Probably these tests need a rebaseline.
Comment 10 Sam Weinig 2011-10-27 11:08:55 PDT
(In reply to comment #9)
> My impression is that the changed hash codes changed the order of items in some tables. Probably these tests need a rebaseline.

Indeed.
Comment 11 Csaba Osztrogonác 2011-10-27 11:42:54 PDT
(In reply to comment #10)
> (In reply to comment #9)
> > My impression is that the changed hash codes changed the order of items in some tables. Probably these tests need a rebaseline.
> 
> Indeed.

http://build.webkit.org/results/SnowLeopard%20Intel%20Release%20%28Tests%29/r98530%20%2834228%29/fast/dom/prototype-inheritance-2-pretty-diff.html

Why is it order problem?
Comment 12 Sam Weinig 2011-10-27 13:29:45 PDT
(In reply to comment #11)
> (In reply to comment #10)
> > (In reply to comment #9)
> > > My impression is that the changed hash codes changed the order of items in some tables. Probably these tests need a rebaseline.
> > 
> > Indeed.
> 
> http://build.webkit.org/results/SnowLeopard%20Intel%20Release%20%28Tests%29/r98530%20%2834228%29/fast/dom/prototype-inheritance-2-pretty-diff.html
> 
> Why is it order problem?

I believe that is just how that test works.  Its results are dependent on what happened earlier in the test.
Comment 13 Michael Saboff 2011-10-27 17:53:02 PDT
(In reply to comment #12)
> (In reply to comment #11)
> > (In reply to comment #10)
> > > (In reply to comment #9)
> > > > My impression is that the changed hash codes changed the order of items in some tables. Probably these tests need a rebaseline.
> > > 
> > > Indeed.
> > 
> > http://build.webkit.org/results/SnowLeopard%20Intel%20Release%20%28Tests%29/r98530%20%2834228%29/fast/dom/prototype-inheritance-2-pretty-diff.html
> > 
> > Why is it order problem?
> 
> I believe that is just how that test works.  Its results are dependent on what happened earlier in the test.

Patch for review with baseline changes to tests in https://bugs.webkit.org/show_bug.cgi?id=71058
(https://bugs.webkit.org/attachment.cgi?id=112792&action=review).