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.

Sign up for GitHub

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

Jump to bottom

Make binary recording memmap efficient III (Avoiding spikes by only reserving memory equal to the trace) #1781

Merged
merged 22 commits into from
Jul 5, 2024
Merged

Make binary recording memmap efficient III (Avoiding spikes by only reserving memory equal to the trace) #1781

merged 22 commits into from
Jul 5, 2024

Conversation

h-mayorquin
Copy link
Collaborator

This is a follow up from #1742, it only allocates opens a memmap pointing to the specific trace and not to the whole file.

@h-mayorquin h-mayorquin added the core Changes to core module label Jul 4, 2023
@h-mayorquin h-mayorquin self-assigned this Jul 4, 2023
@h-mayorquin h-mayorquin changed the title Make binary recording memmap efficient II (Avoiding spikes by only reserving memory equal to the trace) Make binary recording memmap efficient III (Avoiding spikes by only reserving memory equal to the trace) Jul 4, 2023
@h-mayorquin
Copy link
Collaborator Author

Pytest runs all of their tests on the same process and the function that I have measures peak memory within the process so of course this failing. Thinking on how to best handles this.

@h-mayorquin h-mayorquin mentioned this pull request Dec 14, 2023
Closed
@h-mayorquin h-mayorquin mentioned this pull request May 2, 2024
Merged
@h-mayorquin h-mayorquin added the performance Performance issues/improvements label Jun 6, 2024
@h-mayorquin
Copy link
Collaborator Author

OK, with the recent merge of #2796 I got the willpower to finish this off. Test should be passing now and I fixed the merge conflicts.

@alejoe91 @samuelgarcia same drills as in #2796 but this time for reading.

@h-mayorquin h-mayorquin marked this pull request as ready for review June 6, 2024 18:09
@samuelgarcia samuelgarcia added this to the 0.101.0 milestone Jun 12, 2024
@h-mayorquin h-mayorquin mentioned this pull request Jun 20, 2024
Closed
@samuelgarcia
Copy link
Member

@h-mayorquin I guess this is ready for merging.
This is OK for me I made a proposal for the boundaries.

@h-mayorquin
Copy link
Collaborator Author

@samuelgarcia yes, makes sense. It was an old PR : )

@samuelgarcia samuelgarcia merged commit 8a97e10 into SpikeInterface:main Jul 5, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@samuelgarcia samuelgarcia samuelgarcia approved these changes

Assignees

@h-mayorquin h-mayorquin

Labels
core Changes to core module performance Performance issues/improvements
Projects
None yet
Milestone
0.101.0
Development

Successfully merging this pull request may close these issues.
2 participants
@h-mayorquin @samuelgarcia