11–20 of 96 posts
There are a lot of thorny issues with mmap, and I'm sure there are legitimate regressions with the approach and things to be fixed, but it sure would be nice to see an analysis from someone who actually knows what mmap is.
> > I don't think I'm unterstanding this right: You're saying that memory will not be freed by the OS after the process terminates?
> You're understanding it perfectly. The whole raison d'etre for mmap() is the ability to leave stuff in RAM (or swap, albeit if that happens it's completely detrimental to this use case) unlinked from the process itself. Basically it's just storing a file/memory block in RAM which can be accessed from multiple processes.
Wow, does this developer not understand the nuances of mmap().
Why reverting it instead of adding upon it? The author of the revert could easily start working on reintroducing the previous format behind a flag. AFAIK llama.cpp is not even v1 yet. I see this revert PR as unnecessary
Earlier quoted context omitted.
What's ego-driven optimization?
Pushing optimizations for personal glory that look good with some benchmarks but may have unthought of or hidden regressions on other aspects of the code/user base. On the same hand this may be an ego driven revert. from reading the bug it seems like some people might be salty that jart gets a lot of publicity for a few changes where other people which bigger contributions to the project don't get.
Why reverting it instead of adding upon it? The author of the revert could easily start working on reintroducing the previous format behind a flag. AFAIK llama.cpp is not even v1 yet. I see this revert PR as unnecessary
I’m not in the know at all here, but the original PR wasn’t purely additive- there was code deletion, and additions across a number of files. It seems to change the checkpoint format. The code should be abstracted differently for it to be placed behind a flag.
Can someone in the know describe what the hullaballoo is about? Seems like ego-driven optimization that breaks compatibility?
The tl;dr as I understand it is that jart had a misunderstanding of how what was actually happening and the benefits of the map optimization… the claims of actually being able to shrink the model size from 20GB > 6GB were just completely false, and while there was a model loading time improvement, actual memory required and used did not change. A number of people saw this and said that making a breaking change to the…
that's it
I really dislike giving HN exposure to this kind of issue; it only brings us the forbidden pleasure of voyeurism while not helping the maintainers & contributors in the slightest – and can even crystallize conflicts while we eat popcorn. Let us let them take their time, wash their dirty laundry among themselves, and take the time they need to go forward on the project.
It sounds like one person experienced a performance regression as a result of the llama.cpp MMAP changes, and decided to create a pull request to revert all of those changes. While they propose wrapping the mmap changes behind a feature flag / command-line flag, that's not what this PR does -- it just reverts the original commits. It's a "tear it down NOW" reaction rather than a "how can I improve this" reaction.
jart and a few other people in the PR have now proposed a variety of feature-flags or forked versions to address the issue in a more nuanced way.
The argument against the feature flag is ultimately more egregious; it’s an experimental feature, breaks compatibility, decreases memory usage for a fair portion of the population while 10x’ing load speed for the rest so very YMMV for an optimization.
In another project this wouldn‘t even be a revert PR but just a PR to feature flag it. Can’t help but notice that more than half the replies on this PR are from people who have a limited understanding of LLMs, admit to it, and are just adding noise because this project is popular right now. First step would be to lock this contributors only so they can get a more streamlined discussion going.