Skip to content

Update mmap-next.patch #1763

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

Merged
merged 10 commits into from
Nov 16, 2022
Merged

Update mmap-next.patch #1763

merged 10 commits into from
Nov 16, 2022

Conversation

angerman
Copy link
Collaborator

Making a bad hack even worse; but maybe more performant.

Ok, so the whole code in the relevant linker is fairly bad. We set MAP_LOW_MEM if aarch64_HOST_ARCH; or if MAP_32BIT is set.

If MAP_LOW_MEM is defined we define mmap_again. If not it's not even there... there is still some rather odd logic, which likely won't work in some #elif clause, which refers to mmap_again. This is, I believe the whole reason why this thing simply doesn't work properly. But fixing it proper is more involved than just doing this here.

The whole blinker logic needs to be redone really.

Making a bad hack even worse; but maybe more performant.
drop mmap-next
it's not a proper upper bound, but ... at least we won't keep scanning
the same range over and over again
@angerman
Copy link
Collaborator Author

The whole logic here is so bad, it's maddening. The proper solution is likely something along those lines here:

  • figure out where this process is mapped into memory and where the next free page is. Use that as base,
    and allocate enough space for the closure size of the objects we want to load + some wiggle room.
  • Have an allocator assigned to that space, and use it.

This logic might be in the m32_allocator in HEAD.

@angerman angerman marked this pull request as ready for review October 26, 2022 11:57
@angerman
Copy link
Collaborator Author

I guess this is good to go for a squash merge. Still a terrible hack, just a tiny improvement.

@angerman
Copy link
Collaborator Author

So ultimately we now have a bunch of buckets keyed by (addr, size), and we remember the last place we managed to find a free range of pages in that size. Thus we will at most scan the memory linearly once from addr upwards, for each different length size.

One tiny optimisation we could do is, to ensure that a larger key will always be at least as big as a smaller key. But for now this solution is good enough. With 9.2/4/6+ we probably want to switch to the m32 allocator anyway.

@angerman angerman merged commit a88458a into master Nov 16, 2022
@iohk-bors iohk-bors bot deleted the angerman/bad-hack branch November 16, 2022 12:10
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.

1 participant