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

improvement(accountsdb): add mapping in DiskMemoryAllocator that maps a file's mmap-ed memory address to its filename #188

Conversation

aang114
Copy link
Contributor

@aang114 aang114 commented Jul 3, 2024

In this PR, a HashMap field has been added to the DiskMemoryAllocator that maps the mmap-ed memory address of a file to its filename so that the file can be deleted from storage when DiskMemoryAllocator::free() is called.

Note: This PR attempts to resolve part of the issue mentioned in https://github.com/orgs/Syndica/projects/2?pane=issue&itemId=56221388:

rn free(mem_ptr) does nothing - all the cleanup happens on deinit() - if we keep a map(mem_ptr, file_path) then we can properly delete disk memory which we no longer need while the process is running

src/accountsdb/db.zig Outdated Show resolved Hide resolved
@aang114 aang114 changed the title improvement(accountsdb): maintain mapping in DiskMemoryAllocator that maps a file's mmap-ed memory address to its filename improvement(accountsdb): add mapping in DiskMemoryAllocator that maps a file's mmap-ed memory address to its filename Jul 4, 2024
@aang114 aang114 requested a review from InKryption July 4, 2024 22:13
@@ -925,11 +925,15 @@ pub const DiskMemoryAllocator = struct {
count: usize = 0,
mux: std.Thread.Mutex = .{},

/// HashMap that maps the Mmap-ed memory address of a file to the file's index number
hashmap: std.AutoHashMap([*]u8, usize),
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not a huge fan of the fact that this introduces a dependency on another allocator which has to potentially also be thread safe. I would like to instead see an approach where the metadata is placed in (disk) memory before or after the user-facing buffer.

@InKryption InKryption closed this Jul 10, 2024
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.

2 participants