On Thu, 14 Apr 2005, Ingo Molnar wrote:
>
> this patch fixes a memory leak in read-cache.c: when there's cache entry
> collision we should free the previous one.
As you already noticed "read_cache()" normally just populates the
active-cache with pointers to the mmap'ed "active" file.
Whether that is a good idea or not, I do not know. But I do know that the
active file is the single biggest file we work with (ie 1.6MB in size), so
since _most_ tools just read it and modify a very small number of entries,
it seemed like a good idea.
In other words, if the common case is that we update a couple of entries
in the active cache, we actually saved 1.6MB (+ malloc overhead for the 17
_thousand_ allocations) by my approach.
And the leak? There's none. We never actually update an existing entry
that was allocated with malloc(), unless the user does something stupid.
In other words, the only case where there is a "leak" is when the user
does something like
update-cache file file file file file file ..
with the same file listed several times.
And dammit, the whole point of doing stuff in user space is that the
kernel takes care of business. Unlike kernel work, leaking is ok. You just
have to make sure that it is limited enough to to not be a problem. I'm
saying that in this case we're _better_ off leaking, because the mmap()
trick saves us more memory than the leak can ever leak.
(The command line is limited to 128kB or so, which means that the most
files you _can_ add with a single update-cache is _less_ than the mmap
win).
It was _such_ a relief to program in user mode for a change. Not having to
care about the small stuff is wonderful.
Linus
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
