TM: Inline several string functions

RESOLVED FIXED

Status

()

Core
JavaScript Engine
P2
enhancement
RESOLVED FIXED
9 years ago
8 years ago

People

(Reporter: Gregor Wagner, Assigned: gwagner)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 attachment, 4 obsolete attachments)

(Reporter)

Description

9 years ago
Created attachment 391763 [details] [diff] [review]
Patch jsstr.h

A shark session for Chrome Experiments advised to inline some string functions such as isDependent, isFlat, isDeflated....

Comment 1

9 years ago
Comment on attachment 391763 [details] [diff] [review]
Patch jsstr.h

use inline, not JS_ALWAYS_INLINE. This is C++. Otherwise looks good.
(Reporter)

Comment 2

9 years ago
Created attachment 391802 [details] [diff] [review]
Patch jsstr.h
Attachment #391763 - Attachment is obsolete: true

Comment 3

9 years ago
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.
(Reporter)

Comment 4

9 years ago
Created attachment 391803 [details] [diff] [review]
jsstr.h patch
Attachment #391802 - Attachment is obsolete: true
Attachment #391803 - Flags: review?(gal)

Comment 5

9 years ago
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.
(Reporter)

Comment 6

9 years ago
Created attachment 391806 [details] [diff] [review]
Patch jsstr.h
Attachment #391803 - Attachment is obsolete: true
Attachment #391806 - Flags: review?(gal)
Attachment #391803 - Flags: review?(gal)

Comment 7

9 years ago
Comment on attachment 391806 [details] [diff] [review]
Patch jsstr.h

Now we are talking. Thanks!
Attachment #391806 - Flags: review?(gal) → review+

Updated

9 years ago
Keywords: checkin-needed

Updated

9 years ago
Assignee: general → gwagner

Updated

9 years ago
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)

Updated

8 years ago
Assignee: wagnerg → anygregor
(Assignee)

Comment 11

8 years ago
Created attachment 417614 [details] [diff] [review]
refresh
Attachment #391806 - Attachment is obsolete: true
(Assignee)

Updated

8 years ago
Keywords: checkin-needed
Whiteboard: [c-n: tracemonkey] → fixed-in-tracemonkey

Comment 13

8 years ago
http://hg.mozilla.org/mozilla-central/rev/8c66f6fb6730
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.