Bug 91788

Summary: Property storage should grow in reverse address direction, to support butterflies
Product: WebKit Reporter: Filip Pizlo <fpizlo>
Component: JavaScriptCoreAssignee: Filip Pizlo <fpizlo>
Status: RESOLVED FIXED    
Severity: Normal CC: barraclough, fpizlo, ggaren, mhahnenberg, oliver, paroga, zherczeg
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 91796, 92086, 92088    
Bug Blocks: 91933    
Attachments:
Description Flags
it starts
none
more
none
it does more things than before
none
almost done
none
the patch ggaren: review+

Filip Pizlo
Reported 2012-07-19 14:51:28 PDT
This will allow us to implement butterfly objects.
Attachments
it starts (19.25 KB, patch)
2012-07-19 15:14 PDT, Filip Pizlo 		no flags
DetailsFormatted DiffDiff
more (35.05 KB, patch)
2012-07-19 20:43 PDT, Filip Pizlo 		no flags
DetailsFormatted DiffDiff
it does more things than before (37.18 KB, patch)
2012-07-19 22:11 PDT, Filip Pizlo 		no flags
DetailsFormatted DiffDiff
almost done (33.92 KB, patch)
2012-07-21 14:59 PDT, Filip Pizlo 		no flags
DetailsFormatted DiffDiff
the patch (39.58 KB, patch)
2012-07-23 17:29 PDT, Filip Pizlo 		ggaren: review+
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Filip Pizlo
Comment 1 2012-07-19 15:14:37 PDT
Created attachment 153360 [details] it starts
Filip Pizlo
Comment 2 2012-07-19 20:43:49 PDT
Created attachment 153386 [details] more
Filip Pizlo
Comment 3 2012-07-19 22:11:50 PDT
Created attachment 153409 [details] it does more things than before And now, it can even reallocate storage.
Filip Pizlo
Comment 4 2012-07-21 14:55:43 PDT
I initially wanted to reverse both the direction of storage and the direction of PropertyOffsets in one patch, 'cause that seemed more sensible. But when it came to debugging (both performance and soundness), I found that the fact that I had conflated these two changes made life totally miserable. I'm now making only one of the changes (direction of storage) and making it so that if you have to do an out-of-line uncached access, there's some negation that needs to happen. We can make the other change later if we feel like it.
Filip Pizlo
Comment 5 2012-07-21 14:59:57 PDT
Created attachment 153679 [details] almost done Backing up, and putting up for EWS though I suspect the results won't be pretty.
Filip Pizlo
Comment 6 2012-07-21 15:01:18 PDT
Zoltan: this might require some ARM changes because our compact offset patching (for get_by_id) may want to patch in a negative offset. Note that the patch doesn't yet have the right 32-bit code, so it doesn't make sense to start porting yet. But I figured I'd give you a heads up.
Filip Pizlo
Comment 7 2012-07-23 17:29:07 PDT
Created attachment 153912 [details] the patch
Geoffrey Garen
Comment 8 2012-07-23 17:42:21 PDT
Comment on attachment 153912 [details] the patch r=me
Filip Pizlo
Comment 9 2012-07-23 19:13:53 PDT
Landed in http://trac.webkit.org/changeset/123417
Zoltan Herczeg
Comment 10 2012-07-24 02:51:26 PDT
> Zoltan: this might require some ARM changes because our compact offset patching (for get_by_id) may want to patch in a negative offset. Note that the patch doesn't yet have the right 32-bit code, so it doesn't make sense to start porting yet. But I figured I'd give you a heads up. Thanks, I will fix it soon.
Patrick R. Gansterer
Comment 11 2012-07-24 12:19:21 PDT
Landed windows build fix in http://trac.webkit.org/changeset/123503
Note You need to log in before you can comment on or make changes to this bug.