Conversation
| fn get_transmission(&self, transmission_id: TransmissionID<N>) -> Option<Transmission<N>> { | ||
| // Try to get the transmission from the cache first. | ||
| if let Some((transmission, _)) = self.cache_transmissions.lock().get_mut(&transmission_id) { | ||
| if let Some(transmission) = self.cache_transmissions.lock().get(&transmission_id) { |
There was a problem hiding this comment.
In hindsight not sure if this will really have that much impact, the idea is to make the cache more FIFO, because typically older transactions won't be needed in the future anymore.
| /// The LRU cache for `aborted transmission ID` to `certificate IDs` entries that are part of the persistent storage. | ||
| cache_aborted_transmission_ids: Mutex<LruCache<TransmissionID<N>, IndexSet<Field<N>>>>, | ||
| /// The LRU cache for `transmission ID` to `transmission` entries that are part of the persistent storage. | ||
| cache_transmissions: Mutex<LruCache<TransmissionID<N>, Transmission<N>>>, |
There was a problem hiding this comment.
Removed cache_aborted_transmission_ids because we never read from it.
|
Opened an issue about CI failing: #4052 |
node/bft/src/gateway.rs
Outdated
| Err(error) => bail!("[UnconfirmedTransaction] {error}"), | ||
| }; | ||
| // Calculate the transmission checksum. | ||
| let checksum = Data::<Transaction<N>>::Buffer(transaction.to_bytes_le()?.into()).to_checksum::<N>()?; |
There was a problem hiding this comment.
note: this operation involves plenty of redundant work (since we're serializing a transaction that was just deserialized, even if it was in a blob form); it should be possible to optimize it so that the checksum can be calculated based on the event
node/src/validator/router.rs
Outdated
| }; | ||
|
|
||
| // Broadcast the unconfirmed transaction so other validators can cache it. | ||
| self.consensus.bft().primary().gateway().broadcast_unconfirmed_transaction(transaction); |
There was a problem hiding this comment.
we should instead use the serialized transaction here, as it is almost free to clone and write to the socket (as opposed to the deserialized transaction); it could also be used to cheaply construct the transmission id for caching purposes (unless it's already available at the callsite, in which case it would be practical to pass it on to the cache_unconfirmed_transaction call above)
There was a problem hiding this comment.
reducing the redundant id computation we do all over the place is out of scope but looking forward to the day we can do it
|
Looks good to me. I merged |
3 participants
Motivation
Closes #3962
Test Plan
Backwards compatibility