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: randomx memory usage #3301

Merged

Conversation

StriderDM
Copy link
Contributor

@StriderDM StriderDM commented Sep 6, 2021

Description
Removed dataset from RandomXVMInstance, light mode only requires a cache, this fixes a bug where light mode will consume more than 256MB of memory per RandomXVMInstance.

Made RandomXDataset optional for RandomXVMInstance.

resolves #3104
closes #3103

Motivation and Context
Overall memory overhead reduction

How Has This Been Tested?
Manually in a constrained vagrant instance

Screen Shot 2021-09-04 at 12 45 40 AM

Removed dataset from RandomXVMInstance, light mode only requires a cache, this fixes a bug where light mode will consume more than 256MB of memory per RandomXVMInstance.

Made RandomXDataset optional for RandomXVMInstance.

cargo-fmt

Comments
@delta1
Copy link
Contributor

delta1 commented Sep 6, 2021

Nice one! Will test

Copy link
Member

@sdbondi sdbondi left a comment

Choose a reason for hiding this comment

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

Nice - utAck.

EDIT: How are validation times impacted?

Copy link
Collaborator

@stringhandler stringhandler left a comment

Choose a reason for hiding this comment

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

Nice, one question though...

// Note: If a cache and dataset (if assigned) allocated to the VM drops, the VM will crash.
// The cache and dataset for the VM need to be stored together with it since they are not
// mix and match.
instance: Arc<Mutex<(RandomXVM, RandomXCache, Option<RandomXDataset>)>>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why keep the dataset in here if it's always set to None?

@aviator-app aviator-app bot merged commit 52e409d into tari-project:development Sep 6, 2021
Cifko pushed a commit to Cifko/tari that referenced this pull request Sep 10, 2021
Description
Removed dataset from RandomXVMInstance, light mode only requires a cache, this fixes a bug where light mode will consume more than 256MB of memory per RandomXVMInstance.

Made RandomXDataset optional for RandomXVMInstance.

resolves tari-project#3104 
closes tari-project#3103 

Motivation and Context
Overall memory overhead reduction

How Has This Been Tested?
Manually in a constrained vagrant instance

<img width="1048" alt="Screen Shot 2021-09-04 at 12 45 40 AM" src="https://user-images.githubusercontent.com/51991544/132176934-6a5a3753-4960-472c-8544-edebd6eec40d.png">
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.

RandomX VM cannot be created with less than 2GB memory
4 participants