Stop double counting self-referenced classes #4955
Conversation
| if (inBefore && inAfter) continue; | ||
|
|
||
| final object = before.objectsByCodes[code] ?? after.objectsByCodes[code]!; | ||
| final excludeFromRetained = |
There was a problem hiding this comment.
this part is subtle. comment on why the same excludeFromRetained value is used for all cases.
There was a problem hiding this comment.
refactored for readability
There was a problem hiding this comment.
Did you upload the refactor?
What I need is a comment explaining the invariants that make this logic correct.
Is the deal that there should really be a single retained excludedFromRetained set used for both before and after or it it something different?
There was a problem hiding this comment.
if (inBefore && inAfter) continue;
In this code any object is either in before or in after. Never together.
It is obvious for those who read the method from the beginning.
| static ObjectSet empty = ObjectSet()..seal(); | ||
|
|
||
| final objectsByCodes = <IdentityHashCode, AdaptedHeapObject>{}; | ||
| final excludedFromRetained = <IdentityHashCode>{}; |
There was a problem hiding this comment.
nit: is this "excludedFromRetained" or "alreadyIncludedInRetained" or similar?
|
|
||
| final theClass = objects.last.heapClass; | ||
|
|
||
| final firstWithSameClass = |
There was a problem hiding this comment.
This is simpler and I think the intent is a bit clearer if rewritten as:
if (objects.length < 2) return false;
return objects.take(objects.length - 1).any((object) => object.heapClass == theClass);
There was a problem hiding this comment.
take will create one more array
Knowing that the method will run for every object, I would avoid it.
Updated early return.
There was a problem hiding this comment.
actually, I see 'take' returns itarable
| final path = data.retainingPath(objectIndex); | ||
| objects.countInstance( | ||
| object, | ||
| excludeFromRetained: path?.isRetainedBySameClass ?? false, |
jacob314
left a comment
jacob314
left a comment
There was a problem hiding this comment.
Looks good with a couple minor comments.
2 participants
No description provided.