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

Perf/dynamic mm slice adressing #1917

Merged
merged 11 commits into from
Jun 25, 2024
Merged

Conversation

mepatrick73
Copy link
Contributor

Pull Request Template

Checklist

  • Confirmed that run-checks all script has been executed.
  • Made sure the book is up to date with changes in this PR.

Related Issues/PRs

#1752

Changes

Merging of slices was slow (removing from a vector O(n)). Switching to a memory page type of strategy where we index based on the offset of the memory slice. The offset is used to index into the memory page table. Since it's a HashMap removing a slice is done in O(1) time.

Testing

Ran Mnist workload and it improved speed quite a lot.

@mepatrick73 mepatrick73 added the performance Anything related to performance label Jun 21, 2024
@mepatrick73 mepatrick73 self-assigned this Jun 21, 2024
Copy link

codecov bot commented Jun 21, 2024

Codecov Report

Attention: Patch coverage is 43.12500% with 91 lines in your changes missing coverage. Please review.

Project coverage is 85.07%. Comparing base (2fbc462) to head (9bf3b30).

Files Patch % Lines
...-compute/src/memory_management/memory_pool/base.rs 0.00% 90 Missing ⚠️
...-compute/src/memory_management/memory_pool/ring.rs 98.57% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1917      +/-   ##
==========================================
+ Coverage   85.02%   85.07%   +0.04%     
==========================================
  Files         790      790              
  Lines       93275    93256      -19     
==========================================
+ Hits        79310    79335      +25     
+ Misses      13965    13921      -44     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@nathanielsimard nathanielsimard left a comment

Choose a reason for hiding this comment

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

Some minor changes

const MB: usize = 1024 * 1024;
fn main() {
let start = std::time::Instant::now();
println!("Hello world!");
Copy link
Member

Choose a reason for hiding this comment

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

to be removed

Comment on lines 70 to 72
} else {
return false;
}
Copy link
Member

Choose a reason for hiding this comment

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

This else is not really useful

slice_id.copied()
}

fn insert_slice(&mut self, position: usize, slice: &Slice) {
Copy link
Member

Choose a reason for hiding this comment

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

position => address

@nathanielsimard nathanielsimard merged commit 4c90970 into main Jun 25, 2024
15 checks passed
@nathanielsimard nathanielsimard deleted the perf/dynamic-mm-slice-adressing branch June 25, 2024 22:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Anything related to performance
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants