Bug 70937 – Increase StringImpl Flag Bits for 8 bit Strings

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Michael Saboff]

This change is in preparation for adding 8-bit string support.  Two flag bits are needed in StringImpl for 8 bit string support.

[Michael Saboff]

Created attachment 112551 [details]
Proposed patch

[Darin Adler]

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.”

[Michael Saboff]

Committed r98495: <http://trac.webkit.org/changeset/98495>

[Ryosuke Niwa]

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.

[Ryosuke Niwa]

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.

[Ryosuke Niwa]

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 :(

[Csaba Osztrogonác]

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)

[Csaba Osztrogonác]

(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

[Zoltan Herczeg]

My impression is that the changed hash codes changed the order of items in some tables. Probably these tests need a rebaseline.

[Sam Weinig]

(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.

[Csaba Osztrogonác]

(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?

[Sam Weinig]

(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.

[Michael Saboff]

(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).