path: root/trace.c
diff options
authorJeff King <>2019-08-12 20:50:21 (GMT)
committerJunio C Hamano <>2019-08-13 19:21:33 (GMT)
commit9827d4c185e4da728f51cd77c54a38c9de62495f (patch)
tree39b63c919968883f3f9a11e61dec85952e597ba9 /trace.c
parentff66981f4593aec0f3b3eeace0eacb7dbe44fd8c (diff)
packfile: drop release_pack_memory()
Long ago, in 97bfeb34df (Release pack windows before reporting out of memory., 2006-12-24), we taught xmalloc() and friends to try unmapping pack windows when malloc() failed. It's unlikely that his helps a lot in practice, and it has some downsides. First, the downsides: 1. It makes xmalloc() not thread-safe. We've worked around this in pack-objects.c, which installs its own locking version of the try_to_free_routine(). But other threaded code doesn't. 2. It makes the system as a whole harder to reason about. Functions which allocate heap memory under the hood may have farther-reaching effects than expected. That might be worth the tradeoff if there's a benefit. But in practice, it seems unlikely. We're generally dealing with mmap'd files, so the OS is going to do a much better job at responding to memory pressure by dropping individual pages (the exception is systems with NO_MMAP, but even there the OS can probably respond just as well with swapping). So the only thing we're really freeing is address space. On 64-bit systems, we have plenty of that to go around. On 32-bit systems, it could possibly help. But around the same time we made two other changes: 77ccc5bbd1 (Introduce new config option for mmap limit., 2006-12-23) and 60bb8b1453 (Fully activate the sliding window pack access., 2006-12-23). Together that means that a 32-bit system should have no more than 256MB total of packed-git mmaps at one time, split between a few 32MB windows. It's unlikely we have any address space problems since then, but we don't have any data since the features were all added at the same time. Likewise, xmmap() will try to free memory. At first glance, it seems like we'd need this (when we try to mmap a new window, we might need to close an old one to save address space on a 32-bit system). But we're saved again by core.packedGitLimit: if we're going to exceed our 256MB limit, we'll close an existing window before we even call mmap(). So it seems unlikely that this feature is actually doing anything useful. And while we don't have reports of it harming anything (probably because it rarely if ever kicks in), it would be nice to simplify the system overall. This patch drops the whole try_to_free system from xmalloc(), as well as the manual pack memory release in xmmap(). Signed-off-by: Jeff King <> Signed-off-by: Junio C Hamano <>
Diffstat (limited to 'trace.c')
1 files changed, 0 insertions, 2 deletions
diff --git a/trace.c b/trace.c
index fa4a2e7..b3ef0e6 100644
--- a/trace.c
+++ b/trace.c
@@ -88,8 +88,6 @@ static int prepare_trace_line(const char *file, int line,
if (!trace_want(key))
return 0;
- set_try_to_free_routine(NULL); /* is never reset */
/* unit tests may want to disable additional trace output */
if (trace_want(&trace_bare))
return 1;