572820 - TM: de-slot-ify proto

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Andreas Gal :gal]

Its always an object, so we can as well call it by its name.

Comment 1

[Andreas Gal :gal]

Attachment: patch

Comment 2

[Andreas Gal :gal]

Attachment: parent

Comment 3

[Jason Orendorff [:jorendorff]]

Comment on attachment 452042 [details] [diff] [review]
parent

>+LIns*
>+TraceRecorder::stobj_get_proto(nanojit::LIns* obj_ins)
>+{
>+    /* proto is quasi-const, we never change it on trace */
>+    return lir->insLoad(LIR_ldp, obj_ins, offsetof(JSObject, proto), ACC_READONLY);
>+}

I think a call to JS_SetPrototype would change proto without bumping us off trace.

This change is probably safe, but for precarious reasons. I'd feel a
lot more comfortable using alias analysis to pick up this kind of win,
or at the least doing this part in a separate patch from the low-risk
cleanup here.

r=me except for the ACC_READONLY. Awesome patch.

Comment 4

[Brendan Eich [:brendan]]

Write a testcase that assigns to o.__proto__?

Separate bug on making JSSLOT_PARENT opt-in by JSCLASS_HAS_PARENT on file yet?

/be

Comment 5

[Igor Bukanov]

Comment on attachment 452042 [details] [diff] [review]
parent

>diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp
>+/*
>+ * Set object's prototype slot while checking that doing so would not create
>+ * a cycle in the proto chain. The cycle check and proto change are done
>+ * only when all other requests are finished or suspended to ensure exclusive
>+ * access to the chain. If there is a cycle, return false without reporting
>+ * an error. Otherwise, set the proto or parent and return true.
>+ */

Nit: remove the comment about parent.

Comment 6

[Igor Bukanov]

Comment on attachment 452042 [details] [diff] [review]
parent

>+namespace js {
>+
>+bool
>+SetProto(JSContext *cx, JSObject *obj, JSObject *proto, bool checkForCycles)
...
>+    } else if (!js::SetProtoCheckingForCycles(cx, obj, proto)) {
>+        JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, 

Nit: no need for js:: here

Comment 7

[Andreas Gal :gal]

http://hg.mozilla.org/tracemonkey/rev/8eec1f4b6b8a

Comment 8

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/8eec1f4b6b8a