All users were logged out of Bugzilla on October 13th, 2018

Optimize JS_MAX(upcase(localMax), downcase(localMax))

RESOLVED FIXED

Status

()

RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: BijuMailList, Assigned: crowderbt)

Tracking

({perf})

unspecified
Points:
---
Bug Flags:
blocking1.9 +
in-testsuite -
in-litmus -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

1011 bytes, patch
crowderbt
: review+
Details | Diff | Splinter Review
(Reporter)

Description

11 years ago
per bug 412047 Comment #21 - jsregexp.c upcase() is slow
see bug 412047 Comment #16 

optimize JS_MAX() call at 
http://mxr.mozilla.org/mozilla/source/js/src/jsregexp.c#1058
   1057  if (state->flags & JSREG_FOLD) {
   1058      c = (jschar) JS_MAX(upcase(localMax), downcase(localMax));
   1059      if (c > localMax)
   1060          localMax = c;
   1061  }


Similar one I noticed is JS_MIN() call at 
http://mxr.mozilla.org/mozilla/source/js/src/jsobj.h#189
 188 #define LOCKED_OBJ_NSLOTS(obj)
 189    JS_MIN((obj)->map->freeslot, STOBJ_NSLOTS(obj))

I assume compiler will optimize it.
(Reporter)

Updated

11 years ago
Blocks: 412047
(Assignee)

Comment 1

11 years ago
Created attachment 302620 [details] [diff] [review]
easy fix
Assignee: general → crowder
Status: NEW → ASSIGNED
Attachment #302620 - Flags: review?(mrbkap)
(Assignee)

Updated

11 years ago
Keywords: perf
(Assignee)

Updated

11 years ago
Flags: blocking1.9?
(Assignee)

Comment 2

11 years ago
Biju:  Yeah, not worried about the JS_MIN case (though that could bite us if/when we ever convert from MACROS to inline functions).  :(
Comment on attachment 302620 [details] [diff] [review]
easy fix

>? .jsapi.h.swp
>? .jsinterp.c.rej.swp
> ...

Edit this stuff out!

>Index: config.mk

This change is unrelated.

r=mrbkap on the rest of it.
Attachment #302620 - Flags: review?(mrbkap) → review+
(Assignee)

Comment 4

11 years ago
Created attachment 302628 [details] [diff] [review]
cleaned up
Attachment #302620 - Attachment is obsolete: true
Attachment #302628 - Flags: review+
Attachment #302628 - Flags: approval1.9?
(Assignee)

Comment 5

11 years ago
Adding checkin-needed; if/when this patch gets blocking or approval status, land away.
Keywords: checkin-needed

Updated

11 years ago
Flags: blocking1.9? → blocking1.9+
(Assignee)

Comment 6

11 years ago
jsregexp.c:3.180
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
(Assignee)

Updated

11 years ago
Keywords: checkin-needed
Attachment #302628 - Flags: approval1.9?
(Reporter)

Comment 7

11 years ago
Thanks Brian for the fix...

Updated

11 years ago
Flags: in-testsuite-
Flags: in-litmus-
You need to log in before you can comment on or make changes to this bug.