Closed Bug 637280 Opened 9 years ago Closed 9 years ago

avoid AutoKeepAtoms when calling the resolve hook

Categories

(Core :: JavaScript Engine, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: igor, Assigned: igor)

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 file)

Currently CallResolveOp calls AutoKeepAtoms to protect all allocated atoms from the GC during the resolve hook invocation. With the conservative GC scanning the current stack trace this should not be necessary and we can skip the overhead JS_ATOMIC_INCREMENT/JS_ATOMIC_DECREMENT.
Attached patch v1Splinter Review
The patch removes AutoKeepAtoms protection around the resolve hook call. It also simplifies the rest of the function and update comments to reflect the current state of affairs.

With the patch the following benchmark shows 12% improvement when run under the shell with -m option:

// Minimize global object lookup timing for absent properties
this.__proto__ = null;

function test(N) {
    var t = Date.now();
    for (var i = N; i != 0; --i) 
        "not_existing_property" in this;
    return Date.now() - t;
}

test(1e3); // warm up

var results = [];
for (var i = 0; i != 100; ++i)
    results.push(test(1e6));

var sum = 0;
for (var i = 0; i != results.length; ++i)
    sum += results[i];
var average = sum / results.length;
var min = Math.min.apply(null, results);
var max = Math.max.apply(null, results);

print("Min: "+min);
print("Max: "+max);
print("Average: "+average.toFixed(1));
Assignee: general → igor
Comment on attachment 515593 [details] [diff] [review]
v1

In addition to the win above the patch also speedups the browser startup on Linux by 1%.
Attachment #515593 - Flags: review?(jorendorff)
Comment on attachment 515593 [details] [diff] [review]
v1

Nice.
Attachment #515593 - Flags: review?(jorendorff) → review+
http://hg.mozilla.org/tracemonkey/rev/7f65c1c0cc55
Whiteboard: fixed-in-tracemonkey
http://hg.mozilla.org/mozilla-central/rev/7f65c1c0cc55
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.