Page MenuHomePhabricator

[avalanche] Turn ProofRef into a RCUPtr
ClosedPublic

Authored by Fabien on May 14 2022, 00:27.

Details

Reviewers
deadalnix
Group Reviewers
Restricted Project
Commits
rABCcf9bb51b9c91: [avalanche] Turn ProofRef into a RCUPtr
Summary

As per title. This requires to replace the make_shared calls with a call to the RCU factory instead. A move constructor for the proof is needed in order to facilitate deserializing a proof then building the pointer.

Depends on D11509.

Test Plan
ninja all check-all

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Fabien requested review of this revision.May 14 2022, 00:27
deadalnix requested changes to this revision.May 14 2022, 01:32
deadalnix added a subscriber: deadalnix.
deadalnix added inline comments.
src/avalanche/proof.h
142 ↗(On Diff #33520)

How is that not exploding with fireworks? You obviously don't want to do this.

src/rpc/avalanche.cpp
947 ↗(On Diff #33520)

Why do you allocate the proof on stack then move it on heap?

This revision now requires changes to proceed.May 14 2022, 01:32
src/rpc/avalanche.cpp
947 ↗(On Diff #33520)

I'm avoiding the constness from the ProofRef by using some mutable first then moving the data into the ptr to const

deadalnix requested changes to this revision.May 19 2022, 17:27
deadalnix added inline comments.
src/avalanche/processor.cpp
192 ↗(On Diff #33523)

It does looks like to me this is allocated int he wrong place.

199 ↗(On Diff #33523)

The new line should be before this statement, not after.

src/rpc/avalanche.cpp
947 ↗(On Diff #33520)

Why isn't this a problem with shared_ptr?

This revision now requires changes to proceed.May 19 2022, 17:27

Address feedback, move peerData allocation, rebase on top of D11509 and use the converting constructor so avoid the "stack-then-heap" allocation.

This revision is now accepted and ready to land.May 20 2022, 14:52
This revision was automatically updated to reflect the committed changes.