refactor: store peer info objects in shared pointers

[tearfur]

Should fix #6594.

Instead of directly storing the objects in the map nodes and splicing the nodes across different maps, manage their lifecycles using shared pointers.

This PR is the result of me fed up of dealing with the various obscure edge cases that comes with managing the lifecycles of the peer info objects manually. Each of these cases costs me a few days of work just to identify.

[tearfur]

There are still quite a lot of work to be done before this PR can run fast, but right now testers can verify whether the code logic is correct (no crashes, correct peer stats etc.). CC @Coeur

To-do items:

• ~Replace std::unordered_map with a flat map implementation. Unfortunately the current state of small::map is buggy at best, I'm working on the upstream repo to fix that.~ Edit: Done.
• ~Revert #3192. I'm not sure what's the idea behind that PR in the first place, still waiting for Charles to respond to my question over there.~ Edit: Draft PR #6712 created.

[Coeur]

So far so good. I'll let it run for a moment. [edit] no issues in 12 hours

[Coeur]

@tearfur should you mark your draft as ready for review?

[killemov]

A peer is in a peer pool per torrent, correct? So a peer that exists in connection with two different torrents still has/needs two instances, correct? So is using shared pointers replacing the pool structure/functionality or is it extra or is the pool just a map? For me it seems logical to have the peer lifecycle managed by its pool. You request the peer from the pool, you do something with it and inform the pool if you did something lifecycle-changing to the peer.

So:

and splicing the nodes across different maps

concerns me. As in, why would a peer instance be in more than one map anyway?

[tearfur]

This PR deals with peer info objects, not the peers themselves. We use them to store information about a peer that is useful even when we are not currently connected to that peer. So their lifecycles are more complicated than the peers.

[tearfur]

@ckerr Ready for review.

The to-do items in https://github.com/transmission/transmission/pull/6614#issuecomment-1949627038 will be for future PRs.

[tearfur]

@ckerr Please help bump small. The fix was simpler than I expected, so I got it done right away.

• Replace std::unordered_map with a flat map implementation. Unfortunately the current state of small::map is buggy at best, I'm working on the upstream repo to fix that.

^ Now I can resolve the above in this PR.

[tearfur]

Please help bump small. The fix was simpler than I expected, so I got it done right away.

Sorry, another fix is needed before that: https://github.com/alandefreitas/small/pull/44

[ILoveGoulash]

Sorry if this is just some noise, since I don't know anything about TR's code and I'm pretty sure you guys are aware of this "issue" with the std, but are these shared_ptrs accessed a lot but only by the same thread? If yes, it might be interesting to measure the performance loss and if it is significant, think about something like Boost's local_shared_ptr.

I'm only thinking out loud because I've followed your collective odyssey with flamegraphs to improve/restore perf.

[tearfur]

I'm just going to take the performance hit in exchange for stable and reasonably maintainable code.

Plus, if #6712 is also merged, then I don't think the performance hit will be significant.

[tearfur]

Rebased to main.

[tearfur]

DhtTest failure not related to this PR.

[Coeur]

Notes: fix rare crash on peer disconnection