git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Marius Storm-Olsen <mstormo@gmail.com>
To: Johannes Sixt <j6t@kdbg.org>
Cc: mstormo@gmail.com, msysgit@googlegroups.com,
	 patthoyts@googlemail.com, git@vger.kernel.org
Subject: Re: [PATCH] Add custom memory allocator to MinGW and MacOS builds
Date: Sun, 05 Apr 2009 20:43:49 +0200	[thread overview]
Message-ID: <49D8FBE5.9060602@gmail.com> (raw)
In-Reply-To: <200904040835.24377.j6t@kdbg.org>


Johannes Sixt said the following on 04.04.2009 08:35:
> On Samstag, 4. April 2009, Marius Storm-Olsen wrote:
>> Pat Thoyts said the following on 03.04.2009 23:12:
>>> The difference on Windows Vista is that the low fragmentation
>>> heap is the default memory allocator. On Windows XP you need to
>>> enable it specifically for an application. So a possible
>>> alternative to this is just to enable the low fragmentation
>>> heap. (done via GetProcessHeaps and HeapSetInformation Win32
>>> API calls).
>> I know about the low-fragmentation heap, but given that it was
>> only supported on XP and up (and given that I also had MacOSX in
>> mind when considering a custom allocator; see MacOSX got 12%
>> itself ;-), I didn't even consider it. Thanks for clearing up the
>> differences on the Vista and XP benchmarks though! Makes sense.
> 
> Wouldn't a GetProcessHeaps/HeapSetInformation solution add much
> less code, even with a runtime check whether the feature is
> supported?

It certainly would, if you'd also like to simply ignore the extra 
benefit to MacOSX, which was a not-so-bad additional 12%. However, I 
and several of my colleagues also use Git on Mac and see that any 
improvement in the performance there would also be welcome. So, I went 
with the custom allocator approach, instead of just looking into XP.

There might also be other platforms which could benefit from such a 
custom allocator, so I figured that there were many positive sides to 
this, rather than just going for the Low Fragmentation Heap on Windows.


> The improvement that you observed is in a rather special area 
> (repack). How is the improvment in day-to-day tools:
> 
> - procelains used on command line: git-status, git-add, git-commit,
>  git-diff, git-log, perhaps even local git-fetch.
> 
> - plumbing used by guis: git-diff-files, git-diff-tree, git-log, 
> git-rev-parse
> 
> - I'm not even mentioning git-am, git-rebase, because here the time
>  sink is the fork emulation.
> 
> I doubt that the improvement is equally great, and it will perhaps 
> vanish in the noise. 7000+ LOC is a bit much in this case, don't 
> you think so?

I went with repack because it's a lot of data munging, and not so high 
IO. Clearly more I/O intensive git operations would not benefit as 
much as repack. But the goal in this patch was not to speed up I/O.

Obviously there are things that can be, and should be done for the I/O 
side too, but that's a separate subject.

I don't see the 7000+ LOC as such a big deal, given that they are all 
neatly tucked away in a compat subdirectory. They don't even add any 
additional sourcecode to the codepaths, since you just link with it.

Given the benefits, even 5% better than the Low-Frag case for 
single-threaded cases (which is the most dominant in git anyways), it 
think it's reason enough to include it. The 12% boost on Mac should 
also underline this.

Running 'git blame' on one of the files in my repos gives me this result:
XP
     Without nedmalloc: 11.218sec
     With nedmalloc   :  9.514sec (18% speedup)
OSX
     Without nedmalloc: 15.046sec
     With nedmalloc   : 13.957sec (8% speedup)

I'll take those speedups any day :-)


> BTW, I assume that the Boost license is compatible with GPL. But 
> did you check that?

Of course I did, you'll find it under
http://www.fsf.org/licensing/licenses/index_html#GPLCompatibleLicenses

--
.marius

  reply	other threads:[~2009-04-05 18:45 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-04-03 13:52 [PATCH] Add custom memory allocator to MinGW and MacOS builds Marius Storm-Olsen
2009-04-03 14:20 ` Marius Storm-Olsen
2009-04-03 21:12   ` [msysGit] " Pat Thoyts
2009-04-03 23:16     ` Johannes Schindelin
2009-04-03 23:42       ` [msysGit] " Pat Thoyts
2009-04-04  5:16     ` Marius Storm-Olsen
2009-04-04  6:35       ` [msysGit] " Johannes Sixt
2009-04-05 18:43         ` Marius Storm-Olsen [this message]
2009-04-03 15:01 ` Johannes Schindelin
2009-04-03 15:50   ` Janos Laube

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=49D8FBE5.9060602@gmail.com \
    --to=mstormo@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=j6t@kdbg.org \
    --cc=msysgit@googlegroups.com \
    --cc=patthoyts@googlemail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).