WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
117385
fourthTier: DFG should support op_in and it should use patching to make it fast
https://bugs.webkit.org/show_bug.cgi?id=117385
Summary
fourthTier: DFG should support op_in and it should use patching to make it fast
Filip Pizlo
Reported
2013-06-09 13:38:59 PDT
Patch forthcoming.
Attachments
very early work in progress
(7.78 KB, patch)
2013-06-09 13:39 PDT
,
Filip Pizlo
no flags
Details
Formatted Diff
Diff
it should just work
(28.89 KB, patch)
2013-06-10 19:30 PDT
,
Filip Pizlo
no flags
Details
Formatted Diff
Diff
it did just work
(30.94 KB, patch)
2013-06-10 23:00 PDT
,
Filip Pizlo
no flags
Details
Formatted Diff
Diff
the patch
(36.54 KB, patch)
2013-06-11 14:39 PDT
,
Filip Pizlo
no flags
Details
Formatted Diff
Diff
the patch
(50.36 KB, patch)
2013-06-11 18:07 PDT
,
Filip Pizlo
no flags
Details
Formatted Diff
Diff
the patch
(59.04 KB, patch)
2013-06-11 18:23 PDT
,
Filip Pizlo
ggaren
: review+
Details
Formatted Diff
Diff
Show Obsolete
(5)
View All
Add attachment
proposed patch, testcase, etc.
Filip Pizlo
Comment 1
2013-06-09 13:39:43 PDT
Created
attachment 204122
[details]
very early work in progress Still figuring out what the best way of doing this is.
Filip Pizlo
Comment 2
2013-06-10 19:30:56 PDT
Created
attachment 204277
[details]
it should just work
Filip Pizlo
Comment 3
2013-06-10 23:00:12 PDT
Created
attachment 204289
[details]
it did just work Still need to implement 32-bit part, still need to run more tests.
Filip Pizlo
Comment 4
2013-06-11 10:58:19 PDT
Still debugging some LayoutTests/jquery crashes. No perf numbers yet other than microbenchmarks which speed up by a *lot*.
Filip Pizlo
Comment 5
2013-06-11 14:39:08 PDT
Created
attachment 204361
[details]
the patch
Filip Pizlo
Comment 6
2013-06-11 18:07:02 PDT
Created
attachment 204375
[details]
the patch 32-bit now builds and runs clean.
Filip Pizlo
Comment 7
2013-06-11 18:23:32 PDT
Created
attachment 204376
[details]
the patch Added JSRegress tests.
Filip Pizlo
Comment 8
2013-06-12 12:12:05 PDT
This isn't a small speed-up. in-four-cases 297.3037+-2.8573 ^ 21.8263+-0.1147 ^ definitely 13.6213x faster in-one-case-false 178.0126+-2.6962 ^ 10.4301+-0.1371 ^ definitely 17.0673x faster in-one-case-true 156.9623+-2.0516 ^ 10.1108+-0.0654 ^ definitely 15.5242x faster in-two-cases 166.7664+-1.6888 ^ 10.5953+-0.1015 ^ definitely 15.7397x faster
Geoffrey Garen
Comment 9
2013-06-12 13:16:52 PDT
Comment on
attachment 204376
[details]
the patch Would be nice to take even more advantage and further optimize for cases like if (!("property" in object)) throw "oops!"; return object.property; but I can't in good conscience thumb my nose at a 17X speedup.
Filip Pizlo
Comment 10
2013-06-13 09:59:29 PDT
(In reply to
comment #9
)
> (From update of
attachment 204376
[details]
) > Would be nice to take even more advantage and further optimize for cases like > > if (!("property" in object)) > throw "oops!"; > return object.property;
Yes. Here's the plan: 1) Get the baseline JIT to use the DFG's inline caching logic, including for op_in. 2) Have the DFGByteCodeParser extract the inline cache data for op_in the same way it does for op_get_by_id and friends. 3a) For In's that only have one case in the inline cache and they didn't take slow path, simply turn them into CheckStructure and their result becomes a JSConstant(true) or JSConstant(false). 3b) For In's that too two cases (a true and a false case), integrate Branch(In(@object)) with the CFA such that the CFA will remember that on the then case of the Branch, we know that @object has the true structure, and on the else case of the Branch, we know that @object has the false structure. We haven't yet done such things in the CFA, but they are easy to do in our IR; they'll just require a slight refiddling of the relationship between the CFA's executeEffects() method and the mergeXYZ() methods; executeEffects() will have to convey to the merge methods that we will want to merge slightly different data for the different paths of the branch.
> > but I can't in good conscience thumb my nose at a 17X speedup.
Seems sensible! ;-) OTOH, this didn't really speed up pdfjs despite increasing coverage, and despite the fact that pdfjs is clearly making good use of the inline caching - disassembly dumps show lots of juicy DFG In stubs, and most of them look optimal. So I guess those functions that used 'in' simply weren't on the hot path. Oh well it happens. Still happy that I made it fast.
Filip Pizlo
Comment 11
2013-06-13 13:54:57 PDT
Landed in
http://trac.webkit.org/changeset/151569
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug