feat(nns): Highly scalable voting #2932
Conversation
max-dfinity
force-pushed
the
msum/stable-neuron-support-I
branch
from
49fe21f
to
4da5882
Compare
max-dfinity
force-pushed
the
msum/stable-neuron-support-J
branch
from
d113950
to
3e8a6c9
Compare
max-dfinity
force-pushed
the
msum/stable-neuron-support-I
branch
from
4da5882
to
7a82f34
Compare
max-dfinity
force-pushed
the
msum/stable-neuron-support-J
branch
from
af6c045
to
5f91b1f
Compare
| @@ -1105,6 +1109,19 @@ impl ProposalData { | |||
| now_seconds < self.get_deadline_timestamp_seconds(voting_period_seconds) | |||
| } | |||
|
|
|||
| /// Returns if voting is closed along with whether or not any outstanding votes still | |||
| /// need to be cast (when processing following spans multiple messages). | |||
| pub fn voting_is_finished(&self, now_seconds: u64, voting_period_seconds: u64) -> bool { | |||
There was a problem hiding this comment.
Note for reviewer: This is a new concept for proposals, please look at this, along with new proposal status usage.
| let mut actions_performed = 0; | ||
| let check_after_number_actions = 50; |
There was a problem hiding this comment.
It's still unclear (to me) why this is needed, since IIUC checking instructions against a limit should cost 100-1000 instructions and therefore much fewer than any of the operations here. It would be great to avoid those unnecessary complexity if possible.
There was a problem hiding this comment.
Hm - here are the performance numbers when checking every time for tests that are affected:
---------------------------------------------------
Benchmark: cascading_vote_stable_neurons_with_heap_index
total:
instructions: 347.35 M (regressed by 171.95%)
heap_increase: 0 pages (no change)
stable_memory_increase: 128 pages (no change)
---------------------------------------------------
Benchmark: cascading_vote_stable_everything
total:
instructions: 369.54 M (regressed by 146.48%)
heap_increase: 0 pages (no change)
stable_memory_increase: 128 pages (no change)
---------------------------------------------------
Benchmark: centralized_following_all_stable
total:
instructions: 171.33 M (regressed by 415.17%)
heap_increase: 0 pages (no change)
stable_memory_increase: 128 pages (no change)
---------------------------------------------------
Doesn't make a lot of sense, actually, as you'd expect ALL the voting to perform worse, even when using heap, as they all would be checking just as much....
There was a problem hiding this comment.
One clue - instruction_counter() inside canbench doesn't allow you to get an answer relative to the start of all the canbench activity, which means that our checks will be causing it to constantly store and retrieve the state machine in stable storage.
What's weird, however, is that this isn't happening in some of the other cases hitting the same code paths... perhaps they somehow fall under the limit overall, so that the loading/saving of the ProposalVotingStateMachine is avoided.
| @@ -4110,7 +4127,7 @@ impl Governance { | |||
| // to have Open status while it does not accept votes anymore, since | |||
| // the status change happens below this point. | |||
| if proposal.status() == ProposalStatus::Open | |||
| || proposal.accepts_vote(now_seconds, voting_period_seconds) | |||
| || !proposal.voting_is_finished(now_seconds, voting_period_seconds) | |||
There was a problem hiding this comment.
Is it a problem when proposal.latest_tally is not kept up-to-date after a proposal was decided? This might happen when votes are processed on the timer, because proposal.recompute_tally() is not called, after follower processing for a proposal is done.
max-dfinity
force-pushed
the
msum/stable-neuron-support-J
branch
from
73f510a
to
a8c4e18
Compare
This PR adds support for processing neuron following across multiple messages, so that any number of neurons can vote and have their votes processed, regardless of the size and complexity of the neuron following graph or the number of neurons.