Bug 7411 - Inline some functions Shark suggested
Summary: Inline some functions Shark suggested
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Geoffrey Garen
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-02-21 19:49 PST by Geoffrey Garen
Modified: 2006-02-23 11:50 PST (History)
0 users

See Also:


Attachments
patch (10.44 KB, patch)
2006-02-21 19:50 PST, Geoffrey Garen
no flags Details | Formatted Diff | Diff
patch w/formatting to match current guidelines (10.44 KB, patch)
2006-02-21 20:03 PST, Geoffrey Garen
no flags Details | Formatted Diff | Diff
patch to address comments and add a few more inlines (7.03 KB, patch)
2006-02-22 00:25 PST, Geoffrey Garen
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Geoffrey Garen 2006-02-21 19:49:55 PST
2.1% performance gain on JS iBench.
Comment 1 Geoffrey Garen 2006-02-21 19:50:23 PST
Created attachment 6654 [details]
patch
Comment 2 Geoffrey Garen 2006-02-21 20:03:02 PST
Created attachment 6655 [details]
patch w/formatting to match current guidelines

To keep things consistent, I only reformatted when moving a whole class or method.
Comment 3 Darin Adler 2006-02-21 21:43:13 PST
Comment on attachment 6655 [details]
patch w/formatting to match current guidelines

I think that UString::operator= is exactly what hte compiler would generate, inline, if you just didn't declare the operator= at all.

This otherwise looks great, so r=me.
Comment 4 Maciej Stachowiak 2006-02-21 23:35:28 PST
Geoff, does the inline version of ActivationImp::put ever actually get inlined? It is a virtual method, so it could only possibly be inlined if someone called the method scoped to a specific class name so it picks the right one directly. Otherwise it goes through the vtable and can't inline.
Comment 5 Geoffrey Garen 2006-02-22 00:25:41 PST
Created attachment 6660 [details]
patch to address comments and add a few more inlines

Yes, removing operator= altogether is just as well. No, put() does not inline, nor do some other virtuals my first patch inlined. (Also explains why I couldn't get processFuncDecl to inline. That was driving me crazy.)

New patch addresses both issues and adds a few more inlines (which is why I'm asking for review again). Total speedup: 2.9%.
Comment 6 Darin Adler 2006-02-22 09:35:13 PST
Comment on attachment 6660 [details]
patch to address comments and add a few more inlines

r=me
Comment 7 Geoffrey Garen 2006-02-23 11:50:27 PST
Landed r12949.