Preserve all ancestors of nodes that weren't filtered out

[JianFangAtRai]

This PR is to fix the issue Preserve all ancestors of nodes that weren't filtered out(#1). The changes are as follows.

1. removed the logic to remove all ancestors if a node is marked
2. removed the children of a node instead to prevent them from becoming dangling nodes
3. fixed tests

Tested with 11GB of RAI engine heap snapshot and the ancestors were indeed shown up in the containment view in chrome Dev tools.

[Drvi]

Locally, with both master and this PR, when I subsample a snapshot from a fresh Julia session, I can still see results that are not quite right -- even with a relatively fresh Julia session, I can see a lot of <pooled> elements that don't point to anything (but I'd expect them to point to an array) and I can't see any such <pooled> elements in the original snapshot. I think we're not getting it quite right

Also, from a performance and memory pressure perspective, we don't need to create the children_edges dictionary of arrays, which usually allocates many times per node -- instead, we can utilize the fact that each node has its children already laid out in the edges array in order. If we make a cumulative sum of edge count for each node, we can then easily access the edges directly, a la:

# cumsum_edges = cumsum(nodes.edge_count)
function _mark!(seen, queue, node_idx, nodes, cumsum_edges)
    get!(seen, node_idx) do
        cumcnt = cumsum_edges[node_idx]
        prev_cumcnt = get(cumsum_edges, node_idx-1, 0)

        for child_node_idx in @view(nodes.edges.to_pos[prev_cumcnt+1:cumcnt])
            get!(seen, child_node_idx) do
                push!(queue, child_node_idx)
            end
        end
        return nothing
    end
end

[JianFangAtRai]

Locally, with both master and this PR, when I subsample a snapshot from a fresh Julia session, I can still see results that are not quite right -- even with a relatively fresh Julia session, I can see a lot of <pooled> elements that don't point to anything (but I'd expect them to point to an array) and I can't see any such <pooled> elements in the original snapshot. I think we're not getting it quite right

Also, from a performance and memory pressure perspective, we don't need to create the children_edges dictionary of arrays, which usually allocates many times per node -- instead, we can utilize the fact that each node has its children already laid out in the edges array in order. If we make a cumulative sum of edge count for each node, we can then easily access the edges directly, a la:

# cumsum_edges = cumsum(nodes.edge_count)
function _mark!(seen, queue, node_idx, nodes, cumsum_edges)
    get!(seen, node_idx) do
        cumcnt = cumsum_edges[node_idx]
        prev_cumcnt = get(cumsum_edges, node_idx-1, 0)

        for child_node_idx in @view(nodes.edges.to_pos[prev_cumcnt+1:cumcnt])
            get!(seen, child_node_idx) do
                push!(queue, child_node_idx)
            end
        end
        return nothing
    end
end

Will look into it. Thanks

[Drvi]

Actually, maybe the thing with <pooled> not pointing to any array is expected, what we don't want is a deleted <pooled> that was pointing to an Array that is now not reachable... right?

[JianFangAtRai]

Actually, maybe the thing with <pooled> not pointing to any array is expected, what we don't want is a deleted <pooled> that was pointing to an Array that is now not reachable... right?

correct

[Drvi]

Thanks, this looks great! I think it would be great to also add a test that each retained node can be traced to a root, then let's merge!

[JianFangAtRai]

I am going to merge it as it is for now. There are other issues that I found during testing and debugging. I will send out another PR to address them soon.