feat: direct DebugInfo serialization (3/3)

[huitseeker]

We were doing wasteful round-trip conversions in MastForest serialization. The old flow extracted decorator data from centralized storage, formatted it per-node for serialization, then reconstructed centralized storage during deserialization.

Solution

Serialize DebugInfo directly in CSR format. Deserialize it first, then use builders to reconstruct nodes that link to the already-populated decorator storage.

Two distinct insertion paths:

• add_to_forest(): registers decorators (normal assembly, part of MastForestContributor)
• add_to_forest_relaxed(): assumes decorators already in debug_info (deserialization)

Prior Work: Padded Serialization

Earlier PRs in this stack switched to padded operation serialization with batch metadata. Operations and decorator indices are written in their internal padded form for exact reconstruction.

This eliminated the "padding wrinkle" - complex conversions between padded and unpadded representations. Decorator indices in memory now match decorator indices on disk, making direct DebugInfo serialization straightforward.

Fixes #2448.

[bobbinth]

Looks good! Thank you! I left some questions/comments inline.

[bobbinth]

Under what circumstances could this happen? That is, why would there be decorator storage initialized with nodes, but with no decorators?

[huitseeker]

This occurs when nodes exist in the MastForest but have no decorators attached to any operations. We've decided to use a dense representation:

• Serialize all indptr entries sequentially [0, 0, 0, 5, 5, 5, 5, 8, ...], including consecutive equal values that represent empty nodes (dense).
• Versus only serialize non-zero ranges as (node_id, start, end) tuples, skipping empty nodes entirely (sparse).

The sparse encoding overhead (storing node IDs + offsets) exceeds savings from skipping empty node entries — and the dense representation is simpler and more predictable. See the script at https://gist.github.com/huitseeker/7379e2eecffd7020ae577e986057a400 (linked in the PR)

[plafer]

LGTM! MastForest serialization is much cleaner now 🙂

[plafer]

Future PR: this CSR validation logic is copy/pasted for all instances for CSRs that we have. Ideally we would have a CsrMatrix type where we would write that once, and build all or concrete CSR types from it. It would look something like

pub struct CsrMatrix<Data, Idx> {
    data: Vec<Data>,
    indptr: IndexVec<Idx, usize>
}

pub struct NodeToDecoratorIds {
  before_enter_decorators: CsrMatrix<DecoratorId, MastNodeId>,
  after_exit_decorators: CsrMatrix<DecoratorId, MastNodeId>
}

Ideally too we're able to reuse that type to define the "two-level CSR" that OpToDecoratorIds needs.

[huitseeker]

#2545

[bobbinth]

Looks good! Thank you! I left some comments inline. After these are addressed, we should be good to merge.

[bobbinth]

Is there a reason why we want to write the pointers as u32 rather than usize? Is it because usize uses variable-length encoding?

[huitseeker]

They're invalid if >= u32::max. I'll hide that.

[bobbinth]

The usize serialization/deserialization we have will work fine cross-platform as long as the serialized values don't exceed the valid range for usize on the target platform (ref code here).

[huitseeker]

Yes. I edited the comment which was misleading, this is not primarily about platforms (though that's a valid, if minor concern).

I thought that we had a limit (of u32::MAX) on the indices of decorators attached to a given op, and the indices of ops attached to a given node. Indeed the # of ops per node, and the total # of decorators are bound by u32::MAX. I therefore thought there are two equivalent ways of addressing the correctness issue:

• serializing / deserializing as usize, then checking every read value is bound by u32::MAX,
• serializing / deserializing as u32.

The later seemed simpler, allocation aside. I now realize that I am not sure any of those are actually bound by u32::MAX. We at least clearly can have values in op_indptr_for_dec_ids that can exceed u32::MAX, and because they are offsets in that array, same goes for the values of node_indptr_for_op_idx. See other comment.

Thanks for the pushback. Will edit.

[bobbinth]

Our usize serialization is "cross-platform safe" (as long as the actual values stay within 32-bit range, and if not, we have an issue here anyway).

[huitseeker]

if not, we have an issue here anyway

I'm just not sure that's true. Assume for simplicity that you have precisely decorators.len() == u32::MAX / 100 (and that all quantities divide evenly). For each block, do the following:

• name op_idx the index of each operation in the block (we enforce op_idx <= u32::MAX),
• if op_idx is odd, attach all odd-indexed decorators in the decorators array to it,
• symmetrically if op_idx is even, attach all even-indexed decorators in the decorators array to it.

There's no trivial succinct encoding of that map, so decorator_ids.len() is decorators.len() / 2 * num_ops where num_ops is the total number of operations in the MastForest.

Each operation has to index into a u32::MAX / 200-long slice into the decorator_ids array (which values are all u32s). The 201st such slice has indexes ending at an offset > u32::MAX (the start and end indices of that slice are kept in op_indptr_for_dec_ids by definition). Now assume the MastForest has in aggregate over 200 operations — no one has flagrantly broken anything documented about the protocol, yet we have values in op_indptr_for_dec_ids above the u32 range.

It takes a bit more work to see the same issue at work in node_indptr_for_op_idx but it's the same idea.