Bug 7411

Summary:

Inline some functions Shark suggested

Product:

WebKit

Reporter:

Geoffrey Garen <ggaren>

Component:

JavaScriptCore

Assignee:

Geoffrey Garen <ggaren>

Status:

RESOLVED FIXED

Severity:

Normal

Priority:

Version:

420+

Hardware:

Mac

OS:

OS X 10.4

Attachments:

Description	Flags
patch	none
patch w/formatting to match current guidelines	none
patch to address comments and add a few more inlines	darin: review+

Geoffrey Garen

Reported 2006-02-21 19:49:55 PST

2.1% performance gain on JS iBench.

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
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Geoffrey Garen

Comment 1 2006-02-21 19:50:23 PST

Created attachment 6654 [details] patch

Geoffrey Garen

Comment 2 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.

Darin Adler

Comment 3 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.

Maciej Stachowiak

Comment 4 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.

Geoffrey Garen

Comment 5 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%.

Darin Adler

Comment 6 2006-02-22 09:35:13 PST

Comment on attachment 6660 [details] patch to address comments and add a few more inlines r=me

Geoffrey Garen

Comment 7 2006-02-23 11:50:27 PST

Landed r12949.

Note You need to log in before you can comment on or make changes to this bug.