On Fri, Sep 12, 2014 at 04:21:09PM +0200, Michael Haggerty wrote: > > But I still wonder how hard it would be to just remove lock_file structs > > from the global list when they are committed or rolled back. > [...] > > To make that change, we would have to remove entries from the list of > lock_file objects in a way that the code can be interrupted at any time > by a signal while leaving it in a state that is traversable by the > signal handler. > > I think that can be done pretty easily with a singly-linked list. But > with a singly-linked list, we would have to iterate through the list to > find the node that needs to be removed. This could get expensive if > there are a lot of nodes in the list (see below).
Yes, I considered that, but noticed that if we actually cleaned up closed files, the list would not grow to more than a handful of entries. But... > The ref-transaction code is, I think, moving in the direction of > updating all references in a single transaction. This means that we > would need to hold locks for all of the references at once anyway. So it > might be all for naught. That nullifies the whole discussion. Besides the list-traversal thing above, it would mean that we literally _do_ have all of the lockfiles open at once. So cleaning up after ourselves would be nice, but it would not impact the peak memory usage, which would necessarily have one allocated struct per ref. The use of a strbuf is probably a big enough change to save us there. This case was pathological for a few reasons: 1. A ridiculous number of refs in the repository. 2. Touching a large number of them in sequence (via pack-refs). 3. Allocating a 4K buffer per object. For (3), if the average allocation is dropped even to 400 bytes (which would accommodate quite a long pathname), that would reduce the memory usage to ~700MB. Not amazing, but enough not to tip over most modern machines. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
