Marking an object group as having unknown properties should clear the properties list

RESOLVED FIXED in Firefox 62

Status

()

enhancement
P2
normal
RESOLVED FIXED
Last year
Last year

People

(Reporter: jonco, Assigned: jandem)

Tracking

({perf})

unspecified
mozilla62
Points:
---

Firefox Tracking Flags

(firefox62 fixed)

Details

(Whiteboard: [qf])

Attachments

(1 attachment)

As suggested in bug 1449033, ObjectGroup::markUnknown() should clear the properties list.  At the moment it sets the flags to indicate unknown properties but doesn't clear the list itself.
Posted patch PatchSplinter Review
This way we don't sweep/trace/copy these properties for no reason.
Assignee: nobody → jdemooij
Status: NEW → ASSIGNED
Attachment #8975321 - Flags: review?(bhackett1024)
Below is a micro-benchmark, the gc() time improves from 222 ms to 77 ms. Now this is obviously silly because it creates thousands of groups with enough properties that they are marked as unknown, but still, it might improve GC time a bit in the wild.

var objs = [];
function f() {
    var p = {};
    var o = Object.create(p);
    for (var i = 0; i < 130; i++)
        o["x" + i] = i;
    objs.push(o);
}
for (var i = 0; i < 10000; i++)
    f();
var t = new Date;
gc();
print(new Date - t);
Comment on attachment 8975321 [details] [diff] [review]
Patch

bhackett might be on PTO.
Attachment #8975321 - Flags: review?(bhackett1024) → review?(jcoppeard)
Comment on attachment 8975321 [details] [diff] [review]
Patch

Review of attachment 8975321 [details] [diff] [review]:
-----------------------------------------------------------------

This looks fine to me but I'm not an expert on this area and would be more than happy to get Brian's input at some point.

::: js/src/vm/TypeInference.cpp
@@ +4364,5 @@
>  {
> +    // We're about to remove edges from the group to property ids. Incremental
> +    // GC should know about these edges.
> +    if (zone()->needsIncrementalBarrier())
> +        traceChildren(zone()->barrierTracer());

Oh, well spotted!
Attachment #8975321 - Flags: review?(jcoppeard) → review+
Keywords: perf
Priority: -- → P2
Whiteboard: [qf]
Pushed by jandemooij@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/513231568d6e
Clear properties list when marking an object group as having unknown properties. r=jonco
https://hg.mozilla.org/mozilla-central/rev/513231568d6e
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
You need to log in before you can comment on or make changes to this bug.