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

RESOLVED FIXED in Firefox 45

Status

()

Product:
Firefox
Component:
Developer Tools: Memory
RESOLVED FIXED
Reported:
3 years ago
Modified:
3 years ago

People

(Reporter: fitzgen, Assigned: fitzgen)

Tracking

(Blocks: 1 bug)

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

Firefox Tracking Flags

(firefox45 fixed)

Details

Attachments

(1 attachment)

Make DeserializedEdgeRange re-use its referents edge vector
3.48 KB, patch
vporof
: review+
Details | Diff | Splinter Review
(Assignee)

Description

•
3 years ago 
(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
 (Assignee)

Comment 1

•
3 years ago 
Created attachment 8683285 [details] [diff] [review]
Make DeserializedEdgeRange re-use its referents edge vector
Attachment #8683285 - Flags: review?(vporof)

Comment 3

•
3 years ago 
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.