DeserializedEdgeRange should re-use its referents edge vector (not copy it)

RESOLVED FIXED in Firefox 45

Status

()

Firefox
Developer Tools: Memory
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: fitzgen, Assigned: fitzgen)

Tracking

(Blocks: 1 bug)

unspecified
Firefox 45
x86
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox45 fixed)

Details

Attachments

(1 attachment)

(In reply to Jim Blandy :jimb from bug 1024774 comment #165)
> ::: toolkit/devtools/server/DeserializedNode.cpp
> @@ +165,5 @@
> > +          return false;
> > +      }
> > +
> > +      DeserializedNode &referent = node.getEdgeReferent(*edgep);
> > +      edges.append(mozilla::Move(SimpleEdge(name, Node(&referent))));
> 
> It's a pity we have to copy the edge vector like this. Could you file a
> follow-up bug to make this iterate over the DeserializedNode's underlying
> vector, and have 'settle' build a proper JS::ubi::Edge from the current
> element?
Assignee: nobody → nfitzgerald
Has STR: --- → irrelevant
Status: NEW → ASSIGNED
Created attachment 8683285 [details] [diff] [review]
Make DeserializedEdgeRange re-use its referents edge vector
Attachment #8683285 - Flags: review?(vporof)
Comment on attachment 8683285 [details] [diff] [review]
Make DeserializedEdgeRange re-use its referents edge vector

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

This looks solid, but I don't claim to know the existing heapsnapshot code that well. If you're having doubts about whether or not this fits into the grand scheme of things, please check with Jim too.
Attachment #8683285 - Flags: review?(vporof) → review+
Keywords: checkin-needed

Comment 5

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/db82f47b94a2
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox45: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 45
You need to log in before you can comment on or make changes to this bug.