Skip to content

Commit 3aa3266

Browse files
feat(nns): Avoid cloning large fields when listing proposals (dfinity#3505)
# Why When converting from `ProposalData` to `ProposalInfo`, the large fields (e.g. canister WASMs, logos) are "omitted" in a way that those fields are first cloned then cleared. It indeed saves space in the response by not including them, but the instructions are nevertheless wasted for the clone, and this can be significant for certain queries (e.g. listing only proposals including canister WASMs). # What * Make `ProposalInfo`/`ListProposalInfoResponse` pure API types, by removing its corresponding internal type * Add conversion method `proposal_data_to_info` from the `ProposalData` internal type to `ProposalInfo` API type * Implement the equivalent behavior of `omit_large_fields` in the conversion code * Let `Governance::list_proposals` use the new conversion method and return `ListProposalInfoResponse`(API type) * Update benchmark results (98% improvement on list_proposals) # Tests `rs/nns/governance/tests/governance.rs` already tests various cases for `omit_large_fields`
1 parent aa12bb1 commit 3aa3266

32 files changed

+596
-741
lines changed

rs/nns/governance/api/src/ic_nns_governance.pb.v1.rs

Lines changed: 2 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -2158,61 +2158,41 @@ pub struct WaitForQuietState {
21582158
/// This is a view of the ProposalData returned by API queries and is NOT used
21592159
/// for storage. The ballots are restricted to those of the caller's neurons and
21602160
/// additionally it has the computed fields, topic, status, and reward_status.
2161-
#[derive(candid::CandidType, candid::Deserialize, serde::Serialize, comparable::Comparable)]
2162-
#[allow(clippy::derive_partial_eq_without_eq)]
2163-
#[derive(Clone, PartialEq, ::prost::Message)]
2161+
#[derive(candid::CandidType, candid::Deserialize, serde::Serialize, Clone, Debug, PartialEq)]
21642162
pub struct ProposalInfo {
21652163
/// The unique id for this proposal.
2166-
#[prost(message, optional, tag = "1")]
21672164
pub id: Option<::ic_nns_common::pb::v1::ProposalId>,
21682165
/// The ID of the neuron that made this proposal.
2169-
#[prost(message, optional, tag = "2")]
21702166
pub proposer: Option<NeuronId>,
21712167
/// The amount of ICP in E8s to be charged to the proposer if the proposal is
21722168
/// rejected.
2173-
#[prost(uint64, tag = "3")]
21742169
pub reject_cost_e8s: u64,
21752170
/// The proposal originally submitted.
2176-
#[prost(message, optional, tag = "4")]
21772171
pub proposal: Option<Proposal>,
21782172
/// The timestamp, in seconds from the Unix epoch, when this proposal was made.
2179-
#[prost(uint64, tag = "5")]
21802173
pub proposal_timestamp_seconds: u64,
21812174
/// See \[ProposalData::ballots\].
2182-
#[prost(map = "fixed64, message", tag = "6")]
21832175
pub ballots: ::std::collections::HashMap<u64, Ballot>,
21842176
/// See \[ProposalData::latest_tally\].
2185-
#[prost(message, optional, tag = "7")]
21862177
pub latest_tally: Option<Tally>,
21872178
/// See \[ProposalData::decided_timestamp_seconds\].
2188-
#[prost(uint64, tag = "8")]
21892179
pub decided_timestamp_seconds: u64,
21902180
/// See \[ProposalData::executed_timestamp_seconds\].
2191-
#[prost(uint64, tag = "12")]
21922181
pub executed_timestamp_seconds: u64,
21932182
/// See \[ProposalData::failed_timestamp_seconds\].
2194-
#[prost(uint64, tag = "13")]
21952183
pub failed_timestamp_seconds: u64,
21962184
/// See \[ProposalData::failure_reason\].
2197-
#[prost(message, optional, tag = "18")]
21982185
pub failure_reason: Option<GovernanceError>,
21992186
/// See \[ProposalData::reward_event_round\].
2200-
#[prost(uint64, tag = "14")]
22012187
pub reward_event_round: u64,
22022188
/// Derived - see \[Topic\] for more information
2203-
#[prost(enumeration = "Topic", tag = "15")]
22042189
pub topic: i32,
22052190
/// Derived - see \[ProposalStatus\] for more information
2206-
#[prost(enumeration = "ProposalStatus", tag = "16")]
22072191
pub status: i32,
22082192
/// Derived - see \[ProposalRewardStatus\] for more information
2209-
#[prost(enumeration = "ProposalRewardStatus", tag = "17")]
22102193
pub reward_status: i32,
2211-
#[prost(uint64, optional, tag = "19")]
22122194
pub deadline_timestamp_seconds: Option<u64>,
2213-
#[prost(message, optional, tag = "20")]
22142195
pub derived_proposal_information: Option<DerivedProposalInformation>,
2215-
#[prost(uint64, optional, tag = "21")]
22162196
pub total_potential_voting_power: ::core::option::Option<u64>,
22172197
}
22182198

@@ -3479,11 +3459,8 @@ pub struct ListProposalInfo {
34793459
#[prost(bool, optional, tag = "7")]
34803460
pub omit_large_fields: Option<bool>,
34813461
}
3482-
#[derive(candid::CandidType, candid::Deserialize, serde::Serialize, comparable::Comparable)]
3483-
#[allow(clippy::derive_partial_eq_without_eq)]
3484-
#[derive(Clone, PartialEq, ::prost::Message)]
3462+
#[derive(candid::CandidType, candid::Deserialize, serde::Serialize, Clone, Debug, PartialEq)]
34853463
pub struct ListProposalInfoResponse {
3486-
#[prost(message, repeated, tag = "1")]
34873464
pub proposal_info: Vec<ProposalInfo>,
34883465
}
34893466
/// A request to list neurons. The "requested list", i.e., the list of

rs/nns/governance/canbench/canbench_results.yml

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,67 +1,67 @@
11
benches:
22
add_neuron_active_maximum:
33
total:
4-
instructions: 42749552
4+
instructions: 42752796
55
heap_increase: 1
66
stable_memory_increase: 0
77
scopes: {}
88
add_neuron_active_typical:
99
total:
10-
instructions: 2170522
10+
instructions: 2170658
1111
heap_increase: 0
1212
stable_memory_increase: 0
1313
scopes: {}
1414
add_neuron_inactive_maximum:
1515
total:
16-
instructions: 112621390
16+
instructions: 112624375
1717
heap_increase: 1
1818
stable_memory_increase: 0
1919
scopes: {}
2020
add_neuron_inactive_typical:
2121
total:
22-
instructions: 8497159
22+
instructions: 8497036
2323
heap_increase: 0
2424
stable_memory_increase: 0
2525
scopes: {}
2626
cascading_vote_all_heap:
2727
total:
28-
instructions: 35650914
28+
instructions: 35676146
2929
heap_increase: 0
3030
stable_memory_increase: 128
3131
scopes: {}
3232
cascading_vote_heap_neurons_stable_index:
3333
total:
34-
instructions: 61785953
34+
instructions: 61811185
3535
heap_increase: 0
3636
stable_memory_increase: 128
3737
scopes: {}
3838
cascading_vote_stable_everything:
3939
total:
40-
instructions: 188618691
40+
instructions: 188611915
4141
heap_increase: 0
4242
stable_memory_increase: 128
4343
scopes: {}
4444
cascading_vote_stable_neurons_with_heap_index:
4545
total:
46-
instructions: 162350256
46+
instructions: 162343480
4747
heap_increase: 0
4848
stable_memory_increase: 128
4949
scopes: {}
5050
centralized_following_all_stable:
5151
total:
52-
instructions: 78266711
52+
instructions: 78265237
5353
heap_increase: 0
5454
stable_memory_increase: 128
5555
scopes: {}
5656
compute_ballots_for_new_proposal_with_stable_neurons:
5757
total:
58-
instructions: 2223231
58+
instructions: 2223431
5959
heap_increase: 0
6060
stable_memory_increase: 0
6161
scopes: {}
6262
draw_maturity_from_neurons_fund_heap:
6363
total:
64-
instructions: 7643198
64+
instructions: 7656798
6565
heap_increase: 0
6666
stable_memory_increase: 0
6767
scopes: {}
@@ -85,7 +85,7 @@ benches:
8585
scopes: {}
8686
list_neurons_heap:
8787
total:
88-
instructions: 4947095
88+
instructions: 4988540
8989
heap_increase: 9
9090
stable_memory_increase: 0
9191
scopes: {}
@@ -103,13 +103,13 @@ benches:
103103
scopes: {}
104104
list_neurons_stable:
105105
total:
106-
instructions: 113659672
106+
instructions: 113606723
107107
heap_increase: 5
108108
stable_memory_increase: 0
109109
scopes: {}
110110
list_proposals:
111111
total:
112-
instructions: 6477035
112+
instructions: 128658
113113
heap_increase: 0
114114
stable_memory_increase: 0
115115
scopes: {}
@@ -127,37 +127,37 @@ benches:
127127
scopes: {}
128128
neuron_data_validation_heap:
129129
total:
130-
instructions: 406842391
130+
instructions: 406853184
131131
heap_increase: 0
132132
stable_memory_increase: 0
133133
scopes: {}
134134
neuron_data_validation_stable:
135135
total:
136-
instructions: 362640286
136+
instructions: 362648372
137137
heap_increase: 0
138138
stable_memory_increase: 0
139139
scopes: {}
140140
neuron_metrics_calculation_heap:
141141
total:
142-
instructions: 1498269
142+
instructions: 1498869
143143
heap_increase: 0
144144
stable_memory_increase: 0
145145
scopes: {}
146146
neuron_metrics_calculation_stable:
147147
total:
148-
instructions: 3026795
148+
instructions: 3027495
149149
heap_increase: 0
150150
stable_memory_increase: 0
151151
scopes: {}
152152
range_neurons_performance:
153153
total:
154-
instructions: 56448740
154+
instructions: 56447340
155155
heap_increase: 0
156156
stable_memory_increase: 0
157157
scopes: {}
158158
single_vote_all_stable:
159159
total:
160-
instructions: 2805835
160+
instructions: 2805871
161161
heap_increase: 0
162162
stable_memory_increase: 128
163163
scopes: {}

rs/nns/governance/canister/canister.rs

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -774,9 +774,7 @@ fn get_neuron_info_by_id_or_subaccount(
774774
#[query]
775775
fn get_proposal_info(id: ProposalId) -> Option<ProposalInfo> {
776776
debug_log("get_proposal_info");
777-
governance()
778-
.get_proposal_info(&caller(), id)
779-
.map(ProposalInfo::from)
777+
GOVERNANCE.with_borrow(|governance| governance.get_proposal_info(&caller(), id))
780778
}
781779

782780
#[query]
@@ -792,17 +790,13 @@ fn get_neurons_fund_audit_info(
792790
#[query]
793791
fn get_pending_proposals() -> Vec<ProposalInfo> {
794792
debug_log("get_pending_proposals");
795-
governance()
796-
.get_pending_proposals(&caller())
797-
.into_iter()
798-
.map(ProposalInfo::from)
799-
.collect()
793+
GOVERNANCE.with_borrow(|governance| governance.get_pending_proposals(&caller()))
800794
}
801795

802796
#[query]
803797
fn list_proposals(req: ListProposalInfo) -> ListProposalInfoResponse {
804798
debug_log("list_proposals");
805-
governance().list_proposals(&caller(), &(req.into())).into()
799+
GOVERNANCE.with_borrow(|governance| governance.list_proposals(&caller(), &req.into()))
806800
}
807801

808802
#[query]

rs/nns/governance/proto/ic_nns_governance/pb/v1/governance.proto

Lines changed: 0 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -1642,64 +1642,6 @@ message WaitForQuietState {
16421642
uint64 current_deadline_timestamp_seconds = 1;
16431643
}
16441644

1645-
// This is a view of the ProposalData returned by API queries and is NOT used
1646-
// for storage. The ballots are restricted to those of the caller's neurons and
1647-
// additionally it has the computed fields, topic, status, and reward_status.
1648-
message ProposalInfo {
1649-
// The unique id for this proposal.
1650-
ic_nns_common.pb.v1.ProposalId id = 1;
1651-
1652-
// The ID of the neuron that made this proposal.
1653-
ic_nns_common.pb.v1.NeuronId proposer = 2;
1654-
1655-
// The amount of ICP in E8s to be charged to the proposer if the proposal is
1656-
// rejected.
1657-
uint64 reject_cost_e8s = 3;
1658-
1659-
// The proposal originally submitted.
1660-
Proposal proposal = 4;
1661-
1662-
// The timestamp, in seconds from the Unix epoch, when this proposal was made.
1663-
uint64 proposal_timestamp_seconds = 5;
1664-
1665-
// See [ProposalData::ballots].
1666-
map<fixed64, Ballot> ballots = 6;
1667-
1668-
// See [ProposalData::latest_tally].
1669-
Tally latest_tally = 7;
1670-
1671-
// See [ProposalData::decided_timestamp_seconds].
1672-
uint64 decided_timestamp_seconds = 8;
1673-
1674-
// See [ProposalData::executed_timestamp_seconds].
1675-
uint64 executed_timestamp_seconds = 12;
1676-
1677-
// See [ProposalData::failed_timestamp_seconds].
1678-
uint64 failed_timestamp_seconds = 13;
1679-
1680-
// See [ProposalData::failure_reason].
1681-
GovernanceError failure_reason = 18;
1682-
1683-
// See [ProposalData::reward_event_round].
1684-
uint64 reward_event_round = 14;
1685-
1686-
// Derived - see [Topic] for more information
1687-
Topic topic = 15;
1688-
1689-
// Derived - see [ProposalStatus] for more information
1690-
ProposalStatus status = 16;
1691-
1692-
// Derived - see [ProposalRewardStatus] for more information
1693-
ProposalRewardStatus reward_status = 17;
1694-
1695-
optional uint64 deadline_timestamp_seconds = 19;
1696-
1697-
DerivedProposalInformation derived_proposal_information = 20;
1698-
1699-
// See [ProposalData::total_potential_voting_power].
1700-
optional uint64 total_potential_voting_power = 21;
1701-
}
1702-
17031645
// Network economics contains the parameters for several operations related
17041646
// to the economy of the network. When submitting a NetworkEconomics proposal
17051647
// default values (0) are considered unchanged, so a valid proposal only needs
@@ -2492,10 +2434,6 @@ message ListProposalInfo {
24922434
optional bool omit_large_fields = 7;
24932435
}
24942436

2495-
message ListProposalInfoResponse {
2496-
repeated ProposalInfo proposal_info = 1;
2497-
}
2498-
24992437
// A response to "ListKnownNeurons"
25002438
message ListKnownNeuronsResponse {
25012439
// List of known neurons.

0 commit comments

Comments
 (0)