Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: reduce console wallet tui memory usage #3389

Conversation

hansieodendaal
Copy link
Contributor

Description

Transactional information is copied to an internal buffer for the console wallet to use in its screen refresh tick cycle and updates to this information are event-driven. This PR reduces the memory overhead required to keep transactions in their various life cycle stages in the buffer memory.

Motivation and Context

Unnecessary memory was allocated to the internal buffer as the entire completed transaction body was kept while only the signature(s) and maturity is required. This is more significant for large wallets.

How Has This Been Tested?

System-level testing

@hansieodendaal hansieodendaal force-pushed the ho_reduce_tui_memory_usage branch 2 times, most recently from f28e87e to 2bbd04d Compare September 28, 2021 10:49
hansieodendaal added a commit to hansieodendaal/tari that referenced this pull request Sep 28, 2021
hansieodendaal added a commit to hansieodendaal/tari that referenced this pull request Sep 28, 2021
hansieodendaal added a commit to hansieodendaal/tari that referenced this pull request Sep 28, 2021
@hansieodendaal hansieodendaal force-pushed the ho_reduce_tui_memory_usage branch from 2bbd04d to db07817 Compare September 30, 2021 13:32
Copy link
Contributor

@philipr-za philipr-za left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

#[derive(Clone)]
pub struct CompletedTransactionInfo {
pub tx_id: TxId,
pub source_public_key: CommsPublicKey,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can reduce memory usage even more by storing the text representation instead of the crypto primitive

Suggested change
pub source_public_key: CommsPublicKey,
pub source_public_key: String,

pub struct CompletedTransactionInfo {
pub tx_id: TxId,
pub source_public_key: CommsPublicKey,
pub destination_public_key: CommsPublicKey,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

@aviator-app aviator-app bot merged commit ca1e9fd into tari-project:development Oct 4, 2021
@hansieodendaal hansieodendaal deleted the ho_reduce_tui_memory_usage branch October 14, 2021 05:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants