js_ComputeFilename accounts for 10-12% of date-format-tofte

RESOLVED FIXED in Future

Status

()

RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: sayrer, Assigned: igor)

Tracking

({perf})

Trunk
Future
x86
macOS
Points:
---
Bug Flags:
in-testsuite -
in-litmus -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

11 years ago
This test uses eval, and the filename/line# calculations are expensive there.
(Reporter)

Updated

11 years ago
Target Milestone: --- → Future
Version: unspecified → Trunk
(Assignee)

Comment 1

11 years ago
Judging by the code, I think the culprit is js_PCToLineNumber. It walks through all source notes of the script containing the eval to find the note that contains the given pc. A simple fix would be to embed the line number into JSOP_EVAL. 
Assignee: general → igor
(Assignee)

Comment 2

11 years ago
Created attachment 311301 [details] [diff] [review]
v1

The patch introduces JSOP_LINENO and always generates the new bytecode after JSOP_EVAL. I use this independent bytecode and don't extend JSOP_EVAL with an extra 2 bytes to store lineno to avoid extra checks in the shared implementation of JSOP_CALL and JSOP_EVAL.
Attachment #311301 - Flags: review?(shaver)
(Assignee)

Comment 3

11 years ago
Created attachment 311302 [details] [diff] [review]
v1 for real
Attachment #311301 - Attachment is obsolete: true
Attachment #311302 - Flags: review?(shaver)
Attachment #311301 - Flags: review?(shaver)
Comment on attachment 311302 [details] [diff] [review]
v1 for real

r=shaver, indeed.  (The comment above JSXDR_BYTECODE_VERSION should really be changed, since we want to increment the second term, to decrement the expression's value!)
Attachment #311302 - Flags: review?(shaver) → review+
(Assignee)

Comment 5

11 years ago
Comment on attachment 311302 [details] [diff] [review]
v1 for real

Asking for approval: this is a safe optimization to avoid expensive calculations of the line number based on script's pc pointer.
Attachment #311302 - Flags: approval1.9b5?
Attachment #311302 - Flags: approval1.9?
Comment on attachment 311302 [details] [diff] [review]
v1 for real

a1.9b5=beltzner
Attachment #311302 - Flags: approval1.9b5?
Attachment #311302 - Flags: approval1.9b5+
Attachment #311302 - Flags: approval1.9?
Attachment #311302 - Flags: approval1.9+
(Assignee)

Updated

11 years ago
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
(Assignee)

Comment 8

11 years ago
Created attachment 311547 [details] [diff] [review]
using C, not C++ comments

In the checked in patch I have used // FALL THROUGH. This patch fixes this. I will check it in when the tree will be green.

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.