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

Time to replace current allocator? #53

Open
mikrosk opened this issue Mar 16, 2023 · 11 comments
Open

Time to replace current allocator? #53

mikrosk opened this issue Mar 16, 2023 · 11 comments

Comments

@mikrosk
Copy link
Member

mikrosk commented Mar 16, 2023

See https://www.atari-forum.com/viewtopic.php?p=444500&sid=de5eec694a4cabb1cc3b696227a883dc#p444500

While it's clear that the game / engine must be optimised to avoid that many malloc / free calls, maybe it's still worth considering to replace the allocator with something modern / more flexible?

@th-otto
Copy link
Contributor

th-otto commented Mar 17, 2023

Maybe its better to use something on top of malloc, rather tan replacing it. malloc() is a bit special on our platform, because it has to deal with non-contingues allocations from the system.

You could maybe take a look at all the g_slice_ etc functions from glib.

@mikrosk
Copy link
Member Author

mikrosk commented Mar 17, 2023

Actually Doug Lea's allocator is not only quite popular but also quite flexible. Assuming that at least something works on our platform. ;)

As we don't have mmap / munmap calls, I tried to take the path with MORECORE which depends on sbrk. It even allows non-contiguous memory blocks. But when I actually tried to call it, it failed here:

if (HAVE_MORECORE && tbase == CMFAIL) { /* Try noncontiguous MORECORE */
    if (asize < HALF_MAX_SIZE_T) {
      char* br = CMFAIL;
      char* end = CMFAIL;
      ACQUIRE_MALLOC_GLOBAL_LOCK();
      br = (char*)(CALL_MORECORE(asize));
      end = (char*)(CALL_MORECORE(0));    // <--- this returned CMFAIL (-1)
      RELEASE_MALLOC_GLOBAL_LOCK();
      if (br != CMFAIL && end != CMFAIL && br < end) {
        size_t ssize = end - br;
        if (ssize > nb + TOP_FOOT_SIZE) {
          tbase = br;
          tsize = ssize;
        }
      }
    }
  }

Reading through sbrk implementation also doesn't give me much confidence that the function is really compliant with the spec...

@mikrosk
Copy link
Member Author

mikrosk commented Mar 17, 2023

I guess the culprit could be the zero parameter which is not handled in mintlib (basically passed to Malloc as is...): "Calling sbrk() with an increment of 0 can be used to find the current location of the program break."

@th-otto
Copy link
Contributor

th-otto commented Mar 17, 2023

That is because brk()/sbrk() are nonsense with non-continguous memory blocks, you cannot just increase/decrease it. The current implementation just uses it in places where the original would have called sbrk()to get more memory.

And the standard also says
Avoid using brk() and sbrk(): the malloc(3) memory allocation package is the portable and comfortable way of allocating memory

@mikrosk
Copy link
Member Author

mikrosk commented Mar 17, 2023

What about emulating it? Apple does it, we could perhaps too?

@mikrosk
Copy link
Member Author

mikrosk commented Mar 17, 2023

On the other hand, now I get all the shenanigans with _stksize -- it is exactly for that purpose, to divide memory between the stack, current application's heap and other apps/system. So sbrk actually maybe isn't that useless, I need to play with it a bit more.

@th-otto
Copy link
Contributor

th-otto commented Mar 17, 2023

What about emulating it? Apple does it, we could perhaps too?

Look closer. They emulate it by allocating a contingous, virtual area of memory.

@mikrosk
Copy link
Member Author

mikrosk commented Mar 17, 2023

mintlib does basically the same with _heapbase and TPA_INITIALMEM. :) However I found a quick workaround / fix, sbrk(0) can be easily replaced with the previous call's base + allocated size, the allocator is ready for it. So let's do some profiling!

@mikrosk
Copy link
Member Author

mikrosk commented Mar 17, 2023

If you are curious: mikrosk@630015c

... it was rather easy but the time to discover how it works, argh.

Eero has a good way to profile this allocator's performance so I'm curious how it will go. Also, if we decide to go ahead with this one, we'd get memalign and other (broken/missing) functions as a bonus.

@th-otto
Copy link
Contributor

th-otto commented Mar 17, 2023

_heapbase is only set and used when you limit yourself to a fixed size heap. Normally, you don't want to do that, since it either fails if your machine hasn't that much memory available, or fails with out-of-memory when hitting that limit, although the machine has more available. Kind of defeating the purpose of dynamically allocated memory.

@mikrosk
Copy link
Member Author

mikrosk commented Mar 17, 2023

Still, the apple's example is based on the same idea, at least as far as sbrk is concerned.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants