Closed Bug 507531 Opened 15 years ago Closed 15 years ago

TM: Inline several string functions

Categories

(Core :: JavaScript Engine, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED

People

(Reporter: wagnerg, Assigned: gwagner)

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 file, 4 obsolete files)

Attached patch Patch jsstr.h (obsolete) — Splinter Review
A shark session for Chrome Experiments advised to inline some string functions such as isDependent, isFlat, isDeflated....
Comment on attachment 391763 [details] [diff] [review]
Patch jsstr.h

use inline, not JS_ALWAYS_INLINE. This is C++. Otherwise looks good.
Attached patch Patch jsstr.h (obsolete) — Splinter Review
Attachment #391763 - Attachment is obsolete: true
There is a bunch of more JS_INLINE_BLA in there. Could you make those "inline" as well. r=me with that. If you upload a fresh patch I will push it for you.
Attached patch jsstr.h patch (obsolete) — Splinter Review
Attachment #391802 - Attachment is obsolete: true
Attachment #391803 - Flags: review?(gal)
Comment on attachment 391803 [details] [diff] [review]
jsstr.h patch

initFlat and dependentIsPrefix() should be all inline too. initPrefix too. Basically all simple methods in the class, especially if they are defined the header file.
Attached patch Patch jsstr.h (obsolete) — Splinter Review
Attachment #391803 - Attachment is obsolete: true
Attachment #391806 - Flags: review?(gal)
Attachment #391803 - Flags: review?(gal)
Comment on attachment 391806 [details] [diff] [review]
Patch jsstr.h

Now we are talking. Thanks!
Attachment #391806 - Flags: review?(gal) → review+
Keywords: checkin-needed
Assignee: general → gwagner
Severity: normal → enhancement
Priority: -- → P2
Hardware: x86 → All
Should this land in tm or m-c?
Version: unspecified → Trunk
(In reply to comment #8)
> Should this land in tm or m-c?

My understanding is that all Tracemonkey changes land in the TM repo first, and then Sayre merges them across to m-c.
Whiteboard: [c-n: tracemonkey]
Status: NEW → ASSIGNED
And either someone needs to actually land it in tracemonkey, or they need to admit they can't be bothered, and wontfix it.
Assignee: wagnerg → anygregor
Attached patch refreshSplinter Review
Attachment #391806 - Attachment is obsolete: true
Keywords: checkin-needed
Whiteboard: [c-n: tracemonkey] → fixed-in-tracemonkey
http://hg.mozilla.org/mozilla-central/rev/8c66f6fb6730
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: