Apologies for the large change, especially in the first commit, but I didn't see a great way to incrementally remove the C packet.

I was worried that adding an Arc<Packet> to every PacketRc was going to introduce a big performance hit compared to the unsafe inner C packet pointer that we used to use, so I ran the benchmarks:

• TGen: https://github.com/shadow/benchmark-results/tree/master/tgen/2025-01-13-T17-19-39
• Tor: https://github.com/shadow/benchmark-results/tree/master/tor/2025-01-13-T17-54-26

We do see a runtime slowdown effect due to the additional atomic operations as expected.

I'm not sure how bad of a performance hit this is. If we think it's bad, we could consider using RootedRc, but that's makes the code a bit less flexible and I don't remember if we wanted to phase out the use of that module.

I was worried that adding an Arc<Packet> to every PacketRc was going to introduce a big performance hit compared to the unsafe inner C packet pointer that we used to use, so I ran the benchmarks:

• TGen: https://github.com/shadow/benchmark-results/tree/master/tgen/2025-01-13-T17-19-39
• Tor: https://github.com/shadow/benchmark-results/tree/master/tor/2025-01-13-T17-54-26

We do see a runtime slowdown effect due to the additional atomic operations as expected.

I'm not sure I'm convinced that the slowdown is caused by Arcs (or atomics in general). I haven't looked at this PR closely yet, but I ran a benchmark using the C packet with atomic reference counting, and didn't see any significant performance change.

https://github.com/shadow/benchmark-results/tree/master/tor/2025-01-14-T18-06-33

If it is the Arc, maybe we're cloning it much more than we were increasing the refcount of C packets. Or maybe we're doing more memory allocations, or something else.

OK, thanks for checking into that! There's probably something else we could be doing better in the code then, which is good news :)

To get a handle on the performance issue, I set up a simple experiment where a tgen client downloaded 100 MiB from a tgen server. Then I profiled performance using perf by running the following before and after this PR:

perf record --call-graph fp ~/dev/shadow/build/src/main/shadow shadow.yaml

Comparing the before/after diff using perf diff shows the following:

This highlights possible problems with ObjectCounter. Sure enough, it turns out I added a new ObjectCounter to the PacketRc wrapper, but PacketRcs were not being counted previously when it wrapped the C packet. Checking the object counts in the log shows that indeed this could be significant overhead:

Global allocated object counts: {PacketRc:4132234, Packet:148978,

That's 4.1 million new objects being counted just in this simple, small experiment. Removing the PacketRc object counter seems to get rid of the hotspot as perf diff shows:

Next I'll push that change and then run a Tor benchmark to double check.

Removing the object counter from PacketRc has mitigated most of the performance degradation.

Full Tor benchmark results:
https://github.com/shadow/benchmark-results/tree/master/tor/2025-02-13-T22-25-31

This highlights possible problems with ObjectCounter.

Nice find! I'm surprised that the object counter was the problem. I think it's just an extra hash table lookup each time a packet is created/destroyed? I guess there's a lot of packets, but the hash table accesses seem small compared to everything else we do with packets (allocating memory, copying payload data, etc).