{"thread":{"id":"36224","subject":"[PATCH v2] Bump core.deltaBaseCacheLimit to 128MiB","startedAt":"2014-03-19T12:38:32Z","lastAt":"2014-03-21T08:11:19Z","messageCount":14,"participants":["David Kastrup","Junio C Hamano","Duy Nguyen","Jeff King"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"237068","messageId":"1395232712-6412-1-git-send-email-dak@gnu.org","threadId":"36224","inReplyTo":null,"subject":"[PATCH v2] Bump core.deltaBaseCacheLimit to 128MiB","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2014-03-19T12:38:32Z","receivedAt":"2014-03-19T12:38:32Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"The default of 16MiB causes serious thrashing for large delta chains\ncombined with large files.\n\nSigned-off-by: David Kastrup <dak@gnu.org>\n---\n\nForgot the signoff.  For the rationale of this patch and the 128MiB\nchoice, see the original patch.\n\nDocumentation/config.txt | 2 +-\n environment.c            | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 73c8973..1b6950a 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -484,7 +484,7 @@ core.deltaBaseCacheLimit::\n \tto avoid unpacking and decompressing frequently used base\n \tobjects multiple times.\n +\n-Default is 16 MiB on all platforms.  This should be reasonable\n+Default is 128 MiB on all platforms.  This should be reasonable\n for all users/operating systems, except on the largest projects.\n You probably do not need to adjust this value.\n +\ndiff --git a/environment.c b/environment.c\nindex c3c8606..73ed670 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -37,7 +37,7 @@ int core_compression_seen;\n int fsync_object_files;\n size_t packed_git_window_size = DEFAULT_PACKED_GIT_WINDOW_SIZE;\n size_t packed_git_limit = DEFAULT_PACKED_GIT_LIMIT;\n-size_t delta_base_cache_limit = 16 * 1024 * 1024;\n+size_t delta_base_cache_limit = 128 * 1024 * 1024;\n unsigned long big_file_threshold = 512 * 1024 * 1024;\n const char *pager_program;\n int pager_use_color = 1;\n-- \n1.8.3.2\n"},{"id":"237116","messageId":"xmqq38id3nfs.fsf@gitster.dls.corp.google.com","threadId":"36224","inReplyTo":"1395232712-6412-1-git-send-email-dak@gnu.org","subject":"Re: [PATCH v2] Bump core.deltaBaseCacheLimit to 128MiB","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-19T21:09:43Z","receivedAt":"2014-03-19T21:09:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Kastrup <dak@gnu.org> writes:\n\n> The default of 16MiB causes serious thrashing for large delta chains\n> combined with large files.\n>\n> Signed-off-by: David Kastrup <dak@gnu.org>\n> ---\n\nIs that a good argument?  Wouldn't the default of 128MiB burden\nsmaller machines with bloated processes?\n\n> Forgot the signoff.  For the rationale of this patch and the 128MiB\n> choice, see the original patch.\n\n\"See the original patch\", especially written after three-dash lines\nwithout a reference, will not help future readers of \"git log\" who\nlater bisects to find that this change hurt their usage and want to\nsee why it was done unconditionally (as opposed to encouraging those\nwho benefit from this change to configure their Git to use larger\nvalue for them, without hurting others).\n\nWhile I can personally afford 128MiB, I do *not* think 16MiB was\nchosen more scientifically than the choice of 128MiB this change\nproposes to make, and my gut feeling is that this would not have too\nbig a negative impact to anybody, I would prefer to have a reason\nbetter than gut feeling before accepting a default change.\n\nThanks.\n\n\n> Documentation/config.txt | 2 +-\n>  environment.c            | 2 +-\n>  2 files changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index 73c8973..1b6950a 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -484,7 +484,7 @@ core.deltaBaseCacheLimit::\n>  \tto avoid unpacking and decompressing frequently used base\n>  \tobjects multiple times.\n>  +\n> -Default is 16 MiB on all platforms.  This should be reasonable\n> +Default is 128 MiB on all platforms.  This should be reasonable\n>  for all users/operating systems, except on the largest projects.\n>  You probably do not need to adjust this value.\n>  +\n> diff --git a/environment.c b/environment.c\n> index c3c8606..73ed670 100644\n> --- a/environment.c\n> +++ b/environment.c\n> @@ -37,7 +37,7 @@ int core_compression_seen;\n>  int fsync_object_files;\n>  size_t packed_git_window_size = DEFAULT_PACKED_GIT_WINDOW_SIZE;\n>  size_t packed_git_limit = DEFAULT_PACKED_GIT_LIMIT;\n> -size_t delta_base_cache_limit = 16 * 1024 * 1024;\n> +size_t delta_base_cache_limit = 128 * 1024 * 1024;\n>  unsigned long big_file_threshold = 512 * 1024 * 1024;\n>  const char *pager_program;\n>  int pager_use_color = 1;\n"},{"id":"237120","messageId":"87ob11g9st.fsf@fencepost.gnu.org","threadId":"36224","inReplyTo":"xmqq38id3nfs.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2] Bump core.deltaBaseCacheLimit to 128MiB","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2014-03-19T21:25:54Z","receivedAt":"2014-03-19T21:25:54Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> David Kastrup <dak@gnu.org> writes:\n>\n>> The default of 16MiB causes serious thrashing for large delta chains\n>> combined with large files.\n>>\n>> Signed-off-by: David Kastrup <dak@gnu.org>\n>> ---\n>\n> Is that a good argument?  Wouldn't the default of 128MiB burden\n> smaller machines with bloated processes?\n\nThe default file size before Git forgets about delta compression is\n512MiB.  Unpacking 500MiB files with 16MiB of delta storage is going to\nbe uglier.\n\n>> Forgot the signoff.  For the rationale of this patch and the 128MiB\n>> choice, see the original patch.\n>\n> \"See the original patch\", especially written after three-dash lines\n> without a reference, will not help future readers of \"git log\" who\n> later bisects to find that this change hurt their usage and want to\n> see why it was done unconditionally (as opposed to encouraging those\n> who benefit from this change to configure their Git to use larger\n> value for them, without hurting others).\n\nDocumentation/config.txt states:\n\n    core.deltaBaseCacheLimit::\n            Maximum number of bytes to reserve for caching base objects\n            that may be referenced by multiple deltified objects.  By storing the\n            entire decompressed base objects in a cache Git is able\n            to avoid unpacking and decompressing frequently used base\n            objects multiple times.\n    +\n    Default is 16 MiB on all platforms.  This should be reasonable\n    for all users/operating systems, except on the largest projects.\n    You probably do not need to adjust this value.\n\nI've seen this seriously screwing performance in several projects of\nmine that don't really count as \"largest projects\".\n\nSo the description in combination with the current setting is clearly wrong.\n\n> While I can personally afford 128MiB, I do *not* think 16MiB was\n> chosen more scientifically than the choice of 128MiB this change\n> proposes to make, and my gut feeling is that this would not have too\n> big a negative impact to anybody, I would prefer to have a reason\n> better than gut feeling before accepting a default change.\n\nShrug.  I've set my private default anyway, so I don't have anything to\ngain from this change.  If anybody wants to pick this up and put more\nscience into the exact value: be my guest.\n\n-- \nDavid Kastrup\n"},{"id":"237125","messageId":"xmqqlhw5260l.fsf@gitster.dls.corp.google.com","threadId":"36224","inReplyTo":"87ob11g9st.fsf@fencepost.gnu.org","subject":"Re: [PATCH v2] Bump core.deltaBaseCacheLimit to 128MiB","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-19T22:11:22Z","receivedAt":"2014-03-19T22:11:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Kastrup <dak@gnu.org> writes:\n\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>> David Kastrup <dak@gnu.org> writes:\n>>\n>>> The default of 16MiB causes serious thrashing for large delta chains\n>>> combined with large files.\n>>>\n>>> Signed-off-by: David Kastrup <dak@gnu.org>\n>>> ---\n>>\n>> Is that a good argument?  Wouldn't the default of 128MiB burden\n>> smaller machines with bloated processes?\n>\n> The default file size before Git forgets about delta compression is\n> 512MiB.  Unpacking 500MiB files with 16MiB of delta storage is going to\n> be uglier.\n>\n> ...\n>\n> Documentation/config.txt states:\n>\n>     core.deltaBaseCacheLimit::\n>             Maximum number of bytes to reserve for caching base objects\n>             that may be referenced by multiple deltified objects.  By storing the\n>             entire decompressed base objects in a cache Git is able\n>             to avoid unpacking and decompressing frequently used base\n>             objects multiple times.\n>     +\n>     Default is 16 MiB on all platforms.  This should be reasonable\n>     for all users/operating systems, except on the largest projects.\n>     You probably do not need to adjust this value.\n>\n> I've seen this seriously screwing performance in several projects of\n> mine that don't really count as \"largest projects\".\n>\n> So the description in combination with the current setting is clearly wrong.\n\nThat is a good material for proposed log message, and I think you\nare onto something here.\n\nI know that the 512MiB default for the bitFileThreashold (aka\n\"forget about delta compression\") came out of thin air.  It was just\n\"1GB is always too huge for anybody, so let's cut it in half and\ndeclare that value the initial version of a sane threashold\",\nnothing more.\n\nSo it might be that the problem is 512MiB is still too big, relative\nto the 16MiB of delta base cache, and the former may be what needs\nto be tweaked.  If a blob close to but below 512MiB is a problem for\n16MiB delta base cache, it would still be too big to cause the same\nproblem for 128MiB delta base cache---it would evict all the other\nobjects and then end up not being able to fit in the limit itself,\nbusting the limit immediately, no?\n\nI would understand if the change were to update the definition of\ndeltaBaseCacheLimit and link it to the value of bigFileThreashold,\nfor example.  With the presented discussion, I am still not sure if\nwe can say that bumping deltaBaseCacheLimit is the right solution to\nthe \"description with the current setting is clearly wrong\" (which\nis a real issue).\n"},{"id":"237138","messageId":"CACsJy8C3=bz1HmVgQuJRdixMhhb-JKouM7b1L7M047L_4PBViA@mail.gmail.com","threadId":"36224","inReplyTo":"xmqqlhw5260l.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2] Bump core.deltaBaseCacheLimit to 128MiB","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-03-20T01:38:18Z","receivedAt":"2014-03-20T01:38:18Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Mar 20, 2014 at 5:11 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> David Kastrup <dak@gnu.org> writes:\n>\n>> Junio C Hamano <gitster@pobox.com> writes:\n>>\n>>> David Kastrup <dak@gnu.org> writes:\n>>>\n>>>> The default of 16MiB causes serious thrashing for large delta chains\n>>>> combined with large files.\n>>>>\n>>>> Signed-off-by: David Kastrup <dak@gnu.org>\n>>>> ---\n>>>\n>>> Is that a good argument?  Wouldn't the default of 128MiB burden\n>>> smaller machines with bloated processes?\n>>\n>> The default file size before Git forgets about delta compression is\n>> 512MiB.  Unpacking 500MiB files with 16MiB of delta storage is going to\n>> be uglier.\n>>\n>> ...\n>>\n>> Documentation/config.txt states:\n>>\n>>     core.deltaBaseCacheLimit::\n>>             Maximum number of bytes to reserve for caching base objects\n>>             that may be referenced by multiple deltified objects.  By storing the\n>>             entire decompressed base objects in a cache Git is able\n>>             to avoid unpacking and decompressing frequently used base\n>>             objects multiple times.\n>>     +\n>>     Default is 16 MiB on all platforms.  This should be reasonable\n>>     for all users/operating systems, except on the largest projects.\n>>     You probably do not need to adjust this value.\n>>\n>> I've seen this seriously screwing performance in several projects of\n>> mine that don't really count as \"largest projects\".\n>>\n>> So the description in combination with the current setting is clearly wrong.\n>\n> That is a good material for proposed log message, and I think you\n> are onto something here.\n>\n> I know that the 512MiB default for the bitFileThreashold (aka\n> \"forget about delta compression\") came out of thin air.  It was just\n> \"1GB is always too huge for anybody, so let's cut it in half and\n> declare that value the initial version of a sane threashold\",\n> nothing more.\n>\n> So it might be that the problem is 512MiB is still too big, relative\n> to the 16MiB of delta base cache, and the former may be what needs\n> to be tweaked.  If a blob close to but below 512MiB is a problem for\n> 16MiB delta base cache, it would still be too big to cause the same\n> problem for 128MiB delta base cache---it would evict all the other\n> objects and then end up not being able to fit in the limit itself,\n> busting the limit immediately, no?\n>\n> I would understand if the change were to update the definition of\n> deltaBaseCacheLimit and link it to the value of bigFileThreashold,\n> for example.  With the presented discussion, I am still not sure if\n> we can say that bumping deltaBaseCacheLimit is the right solution to\n> the \"description with the current setting is clearly wrong\" (which\n> is a real issue).\n\nI vote make big_file_threshold smaller. 512MB is already unfriendly\nfor many smaller machines. I'm thinking somewhere around 32MB-64MB\n(and maybe increase delta cache base limit to match). The only\ndownside I see is large blobs will be packed  undeltified, which could\nincrease pack size if you have lots of them. But maybe we could\nimprove pack-objects/repack/gc to deltify large blobs anyway if\nthey're old enough.\n-- \nDuy\n"},{"id":"237182","messageId":"xmqqsiqcztu8.fsf@gitster.dls.corp.google.com","threadId":"36224","inReplyTo":"CACsJy8C3=bz1HmVgQuJRdixMhhb-JKouM7b1L7M047L_4PBViA@mail.gmail.com","subject":"Re: [PATCH v2] Bump core.deltaBaseCacheLimit to 128MiB","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-20T17:02:39Z","receivedAt":"2014-03-20T17:02:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen <pclouds@gmail.com> writes:\n\n> On Thu, Mar 20, 2014 at 5:11 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> ...\n>> I know that the 512MiB default for the bitFileThreashold (aka\n>> \"forget about delta compression\") came out of thin air.  It was just\n>> \"1GB is always too huge for anybody, so let's cut it in half and\n>> declare that value the initial version of a sane threashold\",\n>> nothing more.\n>>\n>> So it might be that the problem is 512MiB is still too big, relative\n>> to the 16MiB of delta base cache, and the former may be what needs\n>> to be tweaked.  If a blob close to but below 512MiB is a problem for\n>> 16MiB delta base cache, it would still be too big to cause the same\n>> problem for 128MiB delta base cache---it would evict all the other\n>> objects and then end up not being able to fit in the limit itself,\n>> busting the limit immediately, no?\n>>\n>> I would understand if the change were to update the definition of\n>> deltaBaseCacheLimit and link it to the value of bigFileThreashold,\n>> for example.  With the presented discussion, I am still not sure if\n>> we can say that bumping deltaBaseCacheLimit is the right solution to\n>> the \"description with the current setting is clearly wrong\" (which\n>> is a real issue).\n>\n> I vote make big_file_threshold smaller. 512MB is already unfriendly\n> for many smaller machines. I'm thinking somewhere around 32MB-64MB\n> (and maybe increase delta cache base limit to match).\n\nThese numbers match my gut feeling (e.g. 4k*4k*32-bit uncompressed\nwould be 64MB); delta cash base that is sized to the same as (or\nperhaps twice as big as) that limit may be a good default.\n\n> The only\n> downside I see is large blobs will be packed  undeltified, which could\n> increase pack size if you have lots of them.\n\nI think that is something that can be tweaked, unless the user tells\nus otherwise via command line override, when running the improved\n\"gc --aggressive\" ;-)\n"},{"id":"237210","messageId":"87lhw4er2b.fsf@fencepost.gnu.org","threadId":"36224","inReplyTo":"xmqqsiqcztu8.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2] Bump core.deltaBaseCacheLimit to 128MiB","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2014-03-20T17:08:12Z","receivedAt":"2014-03-20T17:08:12Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Duy Nguyen <pclouds@gmail.com> writes:\n>\n>> The only\n>> downside I see is large blobs will be packed  undeltified, which could\n>> increase pack size if you have lots of them.\n>\n> I think that is something that can be tweaked, unless the user tells\n> us otherwise via command line override, when running the improved\n> \"gc --aggressive\" ;-)\n\ndeltaBaseCacheLimit is used for unpacking, not for packing.\n\n-- \nDavid Kastrup\n"},{"id":"237218","messageId":"xmqqob10wlac.fsf@gitster.dls.corp.google.com","threadId":"36224","inReplyTo":"87lhw4er2b.fsf@fencepost.gnu.org","subject":"Re: [PATCH v2] Bump core.deltaBaseCacheLimit to 128MiB","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-20T22:35:39Z","receivedAt":"2014-03-20T22:35:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Kastrup <dak@gnu.org> writes:\n\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>> Duy Nguyen <pclouds@gmail.com> writes:\n>>\n>>> The only\n>>> downside I see is large blobs will be packed  undeltified, which could\n>>> increase pack size if you have lots of them.\n>>\n>> I think that is something that can be tweaked, unless the user tells\n>> us otherwise via command line override, when running the improved\n>> \"gc --aggressive\" ;-)\n>\n> deltaBaseCacheLimit is used for unpacking, not for packing.\n\nHmm, doesn't packing need to read existing data?\n"},{"id":"237243","messageId":"20140320234859.GD7774@sigill.intra.peff.net","threadId":"36224","inReplyTo":"1395232712-6412-1-git-send-email-dak@gnu.org","subject":"Re: [PATCH v2] Bump core.deltaBaseCacheLimit to 128MiB","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-03-20T23:48:59Z","receivedAt":"2014-03-20T23:48:59Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Mar 19, 2014 at 01:38:32PM +0100, David Kastrup wrote:\n\n> The default of 16MiB causes serious thrashing for large delta chains\n> combined with large files.\n\nDoes it make much sense to bump this without also bumping\nMAX_DELTA_CACHE in sha1_file.c? In my measurements of linux.git, bumping\nthe memory limit did not help much without also bumping the number of\nslots.\n\nI guess that just bumping the memory limit would help with repos which\nhave deltas on large-ish files (whereas the kernel just has a lot of\ndeltas on a lot of little directories), but I'd be curious how much.\n\nIf you have before-and-after numbers for just this patch on some\nrepository, that would be an interesting thing to put in the commit\nmessage.\n\n-Peff\n"},{"id":"237298","messageId":"87y504ccke.fsf@fencepost.gnu.org","threadId":"36224","inReplyTo":"xmqqob10wlac.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2] Bump core.deltaBaseCacheLimit to 128MiB","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2014-03-21T06:04:17Z","receivedAt":"2014-03-21T06:04:17Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> David Kastrup <dak@gnu.org> writes:\n>\n>> Junio C Hamano <gitster@pobox.com> writes:\n>>\n>>> Duy Nguyen <pclouds@gmail.com> writes:\n>>>\n>>>> The only\n>>>> downside I see is large blobs will be packed  undeltified, which could\n>>>> increase pack size if you have lots of them.\n>>>\n>>> I think that is something that can be tweaked, unless the user tells\n>>> us otherwise via command line override, when running the improved\n>>> \"gc --aggressive\" ;-)\n>>\n>> deltaBaseCacheLimit is used for unpacking, not for packing.\n>\n> Hmm, doesn't packing need to read existing data?\n\nJudging from the frequent out-of-memory conditions of git gc\n--aggressive, packing is not restrained by deltaBaseCacheLimit.\n\n-- \nDavid Kastrup\n"},{"id":"237297","messageId":"87txascc6f.fsf@fencepost.gnu.org","threadId":"36224","inReplyTo":"20140320234859.GD7774@sigill.intra.peff.net","subject":"Re: [PATCH v2] Bump core.deltaBaseCacheLimit to 128MiB","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2014-03-21T06:12:40Z","receivedAt":"2014-03-21T06:12:40Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Wed, Mar 19, 2014 at 01:38:32PM +0100, David Kastrup wrote:\n>\n>> The default of 16MiB causes serious thrashing for large delta chains\n>> combined with large files.\n>\n> Does it make much sense to bump this without also bumping\n> MAX_DELTA_CACHE in sha1_file.c? In my measurements of linux.git, bumping\n> the memory limit did not help much without also bumping the number of\n> slots.\n\nIn the cases I checked, bumping MAX_DELTA_CACHE did not help much.\nBumping it once to 512 could be a slight improvement; larger values then\ncaused performance to regress.\n\n> I guess that just bumping the memory limit would help with repos which\n> have deltas on large-ish files (whereas the kernel just has a lot of\n> deltas on a lot of little directories),\n\nWell, those were the most pathological for git-blame so I was somewhat\nfocused on them.\n\n> but I'd be curious how much.\n\nhttp://repo.or.cz/r/wortliste.git\n\nconsists basically of adding lines to a single alphabetically sorted\nfile wortliste of currently size 15MB.\n\n-- \nDavid Kastrup\n"},{"id":"237301","messageId":"CACsJy8DpnfmyDevqBVpJVwSig2QaeQe9EpjdCk92E1zTeTKKyQ@mail.gmail.com","threadId":"36224","inReplyTo":"87y504ccke.fsf@fencepost.gnu.org","subject":"Re: [PATCH v2] Bump core.deltaBaseCacheLimit to 128MiB","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-03-21T07:59:41Z","receivedAt":"2014-03-21T07:59:41Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Mar 21, 2014 at 1:04 PM, David Kastrup <dak@gnu.org> wrote:\n>> Hmm, doesn't packing need to read existing data?\n>\n> Judging from the frequent out-of-memory conditions of git gc\n> --aggressive, packing is not restrained by deltaBaseCacheLimit.\n\npack-objects memory usage is more controlled by pack.windowmemory,\nwhich defaults to unlimited. Does it make sense to default to half\nphysical memory or something?\n-- \nDuy\n"},{"id":"237303","messageId":"87ior8c72z.fsf@fencepost.gnu.org","threadId":"36224","inReplyTo":"xmqqlhw5260l.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2] Bump core.deltaBaseCacheLimit to 128MiB","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2014-03-21T08:02:44Z","receivedAt":"2014-03-21T08:02:44Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> I know that the 512MiB default for the bitFileThreashold (aka\n> \"forget about delta compression\") came out of thin air.  It was just\n> \"1GB is always too huge for anybody, so let's cut it in half and\n> declare that value the initial version of a sane threashold\",\n> nothing more.\n>\n> So it might be that the problem is 512MiB is still too big, relative\n> to the 16MiB of delta base cache, and the former may be what needs\n> to be tweaked.\n\nWell, the point with the 512MiB limit is basically that you can always\nargue \"if you are working with 500MiB files, you will not be using just\n128MiB of main memory anyway\".  The 16MiB limit, in contrast, will get\nutilized for basically every history, even one of small files, that's\ndeep enough.\n\n> If a blob close to but below 512MiB is a problem for 16MiB delta base\n> cache, it would still be too big to cause the same problem for 128MiB\n> delta base cache---it would evict all the other objects and then end\n> up not being able to fit in the limit itself, busting the limit\n> immediately, no?\n\nI think, but may be mistaken, that the delta base limit decides when to\nflush whole blobs.  So a 500MiB file would still be encoded relative to\na single newer version anyway and take memory accordingly.\n\nBut I am not sure about that at all.\n\n-- \nDavid Kastrup\n"},{"id":"237302","messageId":"87eh1wc6oo.fsf@fencepost.gnu.org","threadId":"36224","inReplyTo":"20140320234859.GD7774@sigill.intra.peff.net","subject":"Re: [PATCH v2] Bump core.deltaBaseCacheLimit to 128MiB","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2014-03-21T08:11:19Z","receivedAt":"2014-03-21T08:11:19Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> If you have before-and-after numbers for just this patch on some\n> repository, that would be an interesting thing to put in the commit\n> message.\n\nIt's a hen-and-egg problem regarding the benchmarks.  The most\nimpressive benchmarks arise with the git-blame performance work in\nplace, and the most impressive benchmarks for the git-blame performance\nwork are when this or something similar is in place.  Of course, when\nthere are two really deficient things slowing operations down, fixing\nonly one is going to be much less impressive.\n\nSo I decided to tackle the low-hanging fruit here first.  But it would\nappear that this amounts in far too much work since it means I have to\nsearch for and create some _other_ benchmarking scenario not hampered by\nsubstandard code like the current git-blame is.\n\nI have enough on my plate as it is, so even though it puts the _real_\ngit-blame work in a worse light, I should rather get that finished first\n(nobody will argue to keep the useless threshing of it around).  Of\ncourse, the person then creating the two-line change to\ndeltaBaseCacheLimit will be able to claim much larger performance\nimprovements in his commit message afterwards than what I can claim\nregarding the git-blame work when going first, but that's life.\n\n-- \nDavid Kastrup\n"}]}