From: Stefan Zager <hidden> Date: 2016-06-15 22:59:53
We in the chromium project have a keen interest in adding threading to
git in the pursuit of performance for lengthy operations (checkout,
status, blame, ...). Our motivation comes from hitting some
performance walls when working with repositories the size of chromium
and blink:
https://chromium.googlesource.com/chromium/srchttps://chromium.googlesource.com/chromium/blink
We are particularly concerned with the performance of msysgit, and we
have already chalked up a significant performance gain by turning on
the threading code in pack-objects (which was already enabled for
posix platforms, but not on msysgit, owing to the lack of a correct
pread implementation).
To this end, I'd like to start submitting patches that make the code
base generally more thread-safe and thread-friendly. Right after this
email, I'm going to send the first such patch, which makes the global
list of pack files (packed_git) internal to sha1_file.c.
I realize this may be a contentious topic, and I'd like to get
feedback on the general effort to add more threading to git. I'd
appreciate any feedback you'd like to give up front.
Thanks!
Stefan Zager
From: Robin H. Johnson <hidden> Date: 2016-06-15 22:59:53
On Tue, Feb 11, 2014 at 05:54:51PM -0800, Stefan Zager wrote:
We in the chromium project have a keen interest in adding threading to
git in the pursuit of performance for lengthy operations (checkout,
status, blame, ...). Our motivation comes from hitting some
performance walls when working with repositories the size of chromium
and blink:
+1 from Gentoo on performance improvements for large repos.
The main repository in the ongoing Git migration project looks to be in
the 1.5GB range (and for those that want to propose splitting it up, we
have explored that option and found it lacking), with very deep history
(but no branches of note, and very few tags).
--
Robin Hugh Johnson
Gentoo Linux: Developer, Infrastructure Lead
E-Mail : robbat2@gentoo.org
GnuPG FP : 11ACBA4F 4778E3F6 E4EDF38E B27B944E 34884E85
I have no comments about thread safety improvements (well, not yet).
If you have investigated about git performance on chromium
repositories, could you please sum it up? Threading may be an option
to improve performance, but it's probably not the only option.
--
Duy
On Wed, Feb 12, 2014 at 9:02 AM, Robin H. Johnson [off-list ref] wrote:
On Tue, Feb 11, 2014 at 05:54:51PM -0800, Stefan Zager wrote:
quoted
We in the chromium project have a keen interest in adding threading to
git in the pursuit of performance for lengthy operations (checkout,
status, blame, ...). Our motivation comes from hitting some
performance walls when working with repositories the size of chromium
and blink:
+1 from Gentoo on performance improvements for large repos.
The main repository in the ongoing Git migration project looks to be in
the 1.5GB range (and for those that want to propose splitting it up, we
have explored that option and found it lacking), with very deep history
(but no branches of note, and very few tags).
From v1.9 shallow clone should work for all push/pull/clone... so
history depth does not matter (on the client side). As for
gentoo-x86's large worktree, using index v4 and avoid full-tree
operations (e.g. "status .", not "status"..) should make all
operations reasonably fast. I plan to make "status" fast even without
path limiting with the help of inotify, but that's not going to be
finished soon. Did I miss anything else?
--
Duy
On Wed, Feb 12, 2014 at 9:02 AM, Robin H. Johnson [off-list ref] wrote:
quoted
On Tue, Feb 11, 2014 at 05:54:51PM -0800, Stefan Zager wrote:
quoted
We in the chromium project have a keen interest in adding threading to
git in the pursuit of performance for lengthy operations (checkout,
status, blame, ...). Our motivation comes from hitting some
performance walls when working with repositories the size of chromium
and blink:
+1 from Gentoo on performance improvements for large repos.
The main repository in the ongoing Git migration project looks to be in
the 1.5GB range (and for those that want to propose splitting it up, we
have explored that option and found it lacking), with very deep history
(but no branches of note, and very few tags).
From v1.9 shallow clone should work for all push/pull/clone... so
history depth does not matter (on the client side). As for
gentoo-x86's large worktree, using index v4 and avoid full-tree
operations (e.g. "status .", not "status"..) should make all
operations reasonably fast. I plan to make "status" fast even without
path limiting with the help of inotify, but that's not going to be
finished soon. Did I miss anything else?
Regarding git-status on msysgit, enable core.preloadindex and core.fscache (as of 1.8.5.2).
There's no inotify on Windows, and I gave up using ReadDirectoryChangesW to keep fscache up to date, as it _may_ report DOS file names (e.g. C:\PROGRA~1 instead of C:\Program Files).
From: Erik Faye-Lund <hidden> Date: 2016-06-15 22:59:53
On Wed, Feb 12, 2014 at 2:54 AM, Stefan Zager [off-list ref] wrote:
We in the chromium project have a keen interest in adding threading to
git in the pursuit of performance for lengthy operations (checkout,
status, blame, ...). Our motivation comes from hitting some
performance walls when working with repositories the size of chromium
and blink:
https://chromium.googlesource.com/chromium/srchttps://chromium.googlesource.com/chromium/blink
We are particularly concerned with the performance of msysgit, and we
have already chalked up a significant performance gain by turning on
the threading code in pack-objects (which was already enabled for
posix platforms, but not on msysgit, owing to the lack of a correct
pread implementation).
How did you manage to do this? I'm not aware of any way to implement
pread on Windows (without going down the insanity-path of wrapping and
potentially locking inside every IO operation)...
From: Stefan Zager <hidden> Date: 2016-06-15 22:59:53
On Tue, Feb 11, 2014 at 6:11 PM, Duy Nguyen [off-list ref] wrote:
I have no comments about thread safety improvements (well, not yet).
If you have investigated about git performance on chromium
repositories, could you please sum it up? Threading may be an option
to improve performance, but it's probably not the only option.
Well, the painful operations that we use frequently are pack-objects,
checkout, status, and blame. Anything on Windows that touches a lot
of files is miserable due to the usual file system slowness on
Windows, and luafv.sys (the UAC file virtualization driver) seems to
make it much worse.
With threading turned on, pack-objects on Windows now takes about
twice as long as on Linux, which is still more than a 2x improvement
over the non-threaded operation.
Checkout is really bad on Windows. The blink repository is ~200K
files, and a full clean checkout from the index takes about 10 seconds
on Linux, and about 3:30 on Windows. I used the Very Sleepy profiler
to see where all the time was spent on Windows: 55% of the time was
spent in OpenFile, and 25% in CloseFile (both in win32). My immediate
goal is to add threading to checkout, so those file system calls can
be done in parallel.
Enabling the fscache speeds up status quite a bit. I'm optimistic
that parallelizing the stat calls will yield a further improvement.
Beyond that, it may not be possible to do much more without using a
file system watcher daemon, like facebook does with mercurial.
(https://code.facebook.com/posts/218678814984400/scaling-mercurial-at-facebook/)
Blame is something that chromium and blink developers use heavily, and
it is not unusual for a blame invocation on the blink repository to
run for 30 seconds. It seems like it should be possible to
parallelize blame, but it requires pack file operations to be
thread-safe.
Stefan
From: Stefan Zager <hidden> Date: 2016-06-15 22:59:53
On Tue, Feb 11, 2014 at 7:43 PM, Duy Nguyen [off-list ref] wrote:
From v1.9 shallow clone should work for all push/pull/clone... so
history depth does not matter (on the client side). As for
gentoo-x86's large worktree, using index v4 and avoid full-tree
operations (e.g. "status .", not "status"..) should make all
operations reasonably fast. I plan to make "status" fast even without
path limiting with the help of inotify, but that's not going to be
finished soon. Did I miss anything else?
Chromium developers frequently want to run status over their entire
checkout, and a lot of them run 'git commit -a'. We want to do
everything possible to speed this up.
From: Stefan Zager <hidden> Date: 2016-06-15 22:59:53
On Wed, Feb 12, 2014 at 3:59 AM, Erik Faye-Lund [off-list ref] wrote:
On Wed, Feb 12, 2014 at 2:54 AM, Stefan Zager [off-list ref] wrote:
quoted
We are particularly concerned with the performance of msysgit, and we
have already chalked up a significant performance gain by turning on
the threading code in pack-objects (which was already enabled for
posix platforms, but not on msysgit, owing to the lack of a correct
pread implementation).
How did you manage to do this? I'm not aware of any way to implement
pread on Windows (without going down the insanity-path of wrapping and
potentially locking inside every IO operation)...
I don't want to steal the thunder of my coworker, who wrote the
implementation. He plans to submit it upstream soon-ish. It relies
on using the lpOverlapped argument to ReadFile(), with some additional
tomfoolery to make sure that the implicit position pointer for the
file descriptor doesn't get modified.
Stefan
From: Erik Faye-Lund <hidden> Date: 2016-06-15 22:59:53
On Wed, Feb 12, 2014 at 7:20 PM, Stefan Zager [off-list ref] wrote:
On Wed, Feb 12, 2014 at 3:59 AM, Erik Faye-Lund [off-list ref] wrote:
quoted
On Wed, Feb 12, 2014 at 2:54 AM, Stefan Zager [off-list ref] wrote:
quoted
We are particularly concerned with the performance of msysgit, and we
have already chalked up a significant performance gain by turning on
the threading code in pack-objects (which was already enabled for
posix platforms, but not on msysgit, owing to the lack of a correct
pread implementation).
How did you manage to do this? I'm not aware of any way to implement
pread on Windows (without going down the insanity-path of wrapping and
potentially locking inside every IO operation)...
I don't want to steal the thunder of my coworker, who wrote the
implementation. He plans to submit it upstream soon-ish. It relies
on using the lpOverlapped argument to ReadFile(), with some additional
tomfoolery to make sure that the implicit position pointer for the
file descriptor doesn't get modified.
Is the code available somewhere? I'm especially interested in the
"additional tomfoolery to make sure that the implicit position pointer
for the file descriptor doesn't get modified"-part, as this was what I
ended up butting my head into when trying to do this myself.
From: Stefan Zager <hidden> Date: 2016-06-15 22:59:53
On Wed, Feb 12, 2014 at 10:27 AM, Erik Faye-Lund [off-list ref] wrote:
On Wed, Feb 12, 2014 at 7:20 PM, Stefan Zager [off-list ref] wrote:
quoted
I don't want to steal the thunder of my coworker, who wrote the
implementation. He plans to submit it upstream soon-ish. It relies
on using the lpOverlapped argument to ReadFile(), with some additional
tomfoolery to make sure that the implicit position pointer for the
file descriptor doesn't get modified.
Is the code available somewhere? I'm especially interested in the
"additional tomfoolery to make sure that the implicit position pointer
for the file descriptor doesn't get modified"-part, as this was what I
ended up butting my head into when trying to do this myself.
From: Erik Faye-Lund <hidden> Date: 2016-06-15 22:59:53
On Wed, Feb 12, 2014 at 7:34 PM, Stefan Zager [off-list ref] wrote:
On Wed, Feb 12, 2014 at 10:27 AM, Erik Faye-Lund [off-list ref] wrote:
quoted
On Wed, Feb 12, 2014 at 7:20 PM, Stefan Zager [off-list ref] wrote:
quoted
I don't want to steal the thunder of my coworker, who wrote the
implementation. He plans to submit it upstream soon-ish. It relies
on using the lpOverlapped argument to ReadFile(), with some additional
tomfoolery to make sure that the implicit position pointer for the
file descriptor doesn't get modified.
Is the code available somewhere? I'm especially interested in the
"additional tomfoolery to make sure that the implicit position pointer
for the file descriptor doesn't get modified"-part, as this was what I
ended up butting my head into when trying to do this myself.
On Wed, Feb 12, 2014 at 7:34 PM, Stefan Zager [off-list ref] wrote:
quoted
On Wed, Feb 12, 2014 at 10:27 AM, Erik Faye-Lund [off-list ref] wrote:
quoted
On Wed, Feb 12, 2014 at 7:20 PM, Stefan Zager [off-list ref] wrote:
quoted
I don't want to steal the thunder of my coworker, who wrote the
implementation. He plans to submit it upstream soon-ish. It relies
on using the lpOverlapped argument to ReadFile(), with some additional
tomfoolery to make sure that the implicit position pointer for the
file descriptor doesn't get modified.
Is the code available somewhere? I'm especially interested in the
"additional tomfoolery to make sure that the implicit position pointer
for the file descriptor doesn't get modified"-part, as this was what I
ended up butting my head into when trying to do this myself.
From: Stefan Zager <hidden> Date: 2016-06-15 22:59:53
On Wed, Feb 12, 2014 at 11:22 AM, Karsten Blees [off-list ref] wrote:
Am 12.02.2014 19:37, schrieb Erik Faye-Lund:
quoted
On Wed, Feb 12, 2014 at 7:34 PM, Stefan Zager [off-list ref] wrote:
quoted
On Wed, Feb 12, 2014 at 10:27 AM, Erik Faye-Lund [off-list ref] wrote:
quoted
On Wed, Feb 12, 2014 at 7:20 PM, Stefan Zager [off-list ref] wrote:
quoted
I don't want to steal the thunder of my coworker, who wrote the
implementation. He plans to submit it upstream soon-ish. It relies
on using the lpOverlapped argument to ReadFile(), with some additional
tomfoolery to make sure that the implicit position pointer for the
file descriptor doesn't get modified.
Is the code available somewhere? I'm especially interested in the
"additional tomfoolery to make sure that the implicit position pointer
for the file descriptor doesn't get modified"-part, as this was what I
ended up butting my head into when trying to do this myself.
From: Mike Hommey <hidden> Date: 2016-06-15 22:59:54
On Wed, Feb 12, 2014 at 12:00:19PM +0100, Karsten Blees wrote:
Am 12.02.2014 04:43, schrieb Duy Nguyen:
quoted
On Wed, Feb 12, 2014 at 9:02 AM, Robin H. Johnson [off-list ref] wrote:
quoted
On Tue, Feb 11, 2014 at 05:54:51PM -0800, Stefan Zager wrote:
quoted
We in the chromium project have a keen interest in adding threading to
git in the pursuit of performance for lengthy operations (checkout,
status, blame, ...). Our motivation comes from hitting some
performance walls when working with repositories the size of chromium
and blink:
+1 from Gentoo on performance improvements for large repos.
The main repository in the ongoing Git migration project looks to be in
the 1.5GB range (and for those that want to propose splitting it up, we
have explored that option and found it lacking), with very deep history
(but no branches of note, and very few tags).
From v1.9 shallow clone should work for all push/pull/clone... so
history depth does not matter (on the client side). As for
gentoo-x86's large worktree, using index v4 and avoid full-tree
operations (e.g. "status .", not "status"..) should make all
operations reasonably fast. I plan to make "status" fast even without
path limiting with the help of inotify, but that's not going to be
finished soon. Did I miss anything else?
Regarding git-status on msysgit, enable core.preloadindex and core.fscache (as of 1.8.5.2).
There's no inotify on Windows, and I gave up using ReadDirectoryChangesW to
keep fscache up to date, as it _may_ report DOS file names (e.g. C:\PROGRA~1
instead of C:\Program Files).
You can use GetLongPathNameW to get the latter from the former.
Mike
On Wed, Feb 12, 2014 at 12:00:19PM +0100, Karsten Blees wrote:
quoted
Am 12.02.2014 04:43, schrieb Duy Nguyen:
quoted
On Wed, Feb 12, 2014 at 9:02 AM, Robin H. Johnson [off-list ref] wrote:
quoted
On Tue, Feb 11, 2014 at 05:54:51PM -0800, Stefan Zager wrote:
quoted
We in the chromium project have a keen interest in adding threading to
git in the pursuit of performance for lengthy operations (checkout,
status, blame, ...). Our motivation comes from hitting some
performance walls when working with repositories the size of chromium
and blink:
+1 from Gentoo on performance improvements for large repos.
The main repository in the ongoing Git migration project looks to be in
the 1.5GB range (and for those that want to propose splitting it up, we
have explored that option and found it lacking), with very deep history
(but no branches of note, and very few tags).
From v1.9 shallow clone should work for all push/pull/clone... so
history depth does not matter (on the client side). As for
gentoo-x86's large worktree, using index v4 and avoid full-tree
operations (e.g. "status .", not "status"..) should make all
operations reasonably fast. I plan to make "status" fast even without
path limiting with the help of inotify, but that's not going to be
finished soon. Did I miss anything else?
Regarding git-status on msysgit, enable core.preloadindex and core.fscache (as of 1.8.5.2).
There's no inotify on Windows, and I gave up using ReadDirectoryChangesW to
keep fscache up to date, as it _may_ report DOS file names (e.g. C:\PROGRA~1
instead of C:\Program Files).
You can use GetLongPathNameW to get the latter from the former.
Mike
Except if its a delete or rename notification...my final ReadDirectoryChangesW version cached the files by their long _and_ short names, but was so complex that it slowed most commands down rather than speeding them up :-)
From: brian m. carlson <hidden> Date: 2016-06-15 22:59:54
On Tue, Feb 11, 2014 at 05:54:51PM -0800, Stefan Zager wrote:
To this end, I'd like to start submitting patches that make the code
base generally more thread-safe and thread-friendly. Right after this
email, I'm going to send the first such patch, which makes the global
list of pack files (packed_git) internal to sha1_file.c.
I'm definitely interested in this if it also works on POSIX systems. At
work, we have a 7.6 GiB repo (packed)[0], so while performance is not
bad, I certainly wouldn't object if it were better.
[0] Using du -sh. For comparison, the Linux kernel repo is 1.4 GiB.
--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187
From: Stefan Zager <hidden> Date: 2016-06-15 22:59:54
On Thu, Feb 13, 2014 at 12:27 AM, Johannes Sixt [off-list ref] wrote:
Am 2/12/2014 20:30, schrieb Stefan Zager:
quoted
On Wed, Feb 12, 2014 at 11:22 AM, Karsten Blees [off-list ref] wrote:
quoted
Am 12.02.2014 19:37, schrieb Erik Faye-Lund:
quoted
ReOpenFile, that's fantastic. Thanks a lot!
...but should be loaded dynamically via GetProcAddress, or are we ready to drop XP support?
Right, that is an issue. From our perspective, it's well past time to
drop XP support.
Not from mine.
All this really means is that the build config will test WIN_VER, and
there will need to be an additional binary distribution of msysgit for
newer Windows.
On Wed, Feb 12, 2014 at 7:34 PM, Stefan Zager <szager <at> google.com>
wrote:
quoted
quoted
On Wed, Feb 12, 2014 at 10:27 AM, Erik Faye-Lund <kusmabite <at>
gmail.com> wrote:
quoted
quoted
quoted
On Wed, Feb 12, 2014 at 7:20 PM, Stefan Zager <szager <at> google.com>
wrote:
quoted
quoted
quoted
quoted
I don't want to steal the thunder of my coworker, who wrote the
implementation. He plans to submit it upstream soon-ish. It relies
on using the lpOverlapped argument to ReadFile(), with some
additional
quoted
quoted
quoted
quoted
tomfoolery to make sure that the implicit position pointer for the
file descriptor doesn't get modified.
Is the code available somewhere? I'm especially interested in the
"additional tomfoolery to make sure that the implicit position pointer
for the file descriptor doesn't get modified"-part, as this was what I
ended up butting my head into when trying to do this myself.
...but should be loaded dynamically via GetProcAddress, or are we ready to
drop XP support?
Original patch author here. In trying to prepare this patch to use
GetProcAddress to load dynamically, I've run into a bit of a snag.
NO_THREAD_SAFE_PREAD is a compile-time flag, which will be incompatible with
any attempt to make this a runtime decision a la LoadLibrary /
GetProcAddress. On XP, we would need to fallback to the single-threaded
path, and on Vista+ we would use the thread-able path, and obviously this
decision could not be made until runtime.
If MinGW were the only configuration using NO_THREAD_SAFE_PREAD, I would
just remove it entirely, but it appears Cygwin configuration uses it also.
Suggestions?
One possibility is to disallow (by convention, perhaps), the use of pread()
and read() against the same fd. The only reason ReOpenFile is necessary at
all is because some code somewhere is mixing read-styles against the same
fd.
The only reason ReOpenFile is necessary at
all is because some code somewhere is mixing read-styles against the same
fd.
I don't understand...ReadFile with OVERLAPPED parameter doesn't modify the HANDLE's file position, so you should be able to mix read()/pread() however you like (as long as read() is only called from one thread).
I tried without ReOpenFile and it seems to work like a charm, or am I missing something?
----8<----
ssize_t mingw_pread(int fd, void *buf, size_t count, off64_t offset)
{
DWORD bytes_read;
OVERLAPPED overlapped;
memset(&overlapped, 0, sizeof(overlapped));
overlapped.Offset = (DWORD) offset;
overlapped.OffsetHigh = (DWORD) (offset >> 32);
if (!ReadFile((HANDLE) _get_osfhandle(fd), buf, count, &bytes_read,
&overlapped)) {
errno = err_win_to_posix(GetLastError());
return -1;
}
return (ssize_t) bytes_read;
}
From: Stefan Zager <hidden> Date: 2016-06-15 22:59:54
On Thu, Feb 13, 2014 at 2:51 PM, Karsten Blees [off-list ref] wrote:
Am 13.02.2014 19:38, schrieb Zachary Turner:
quoted
The only reason ReOpenFile is necessary at
all is because some code somewhere is mixing read-styles against the same
fd.
I don't understand...ReadFile with OVERLAPPED parameter doesn't modify the HANDLE's file position, so you should be able to mix read()/pread() however you like (as long as read() is only called from one thread).
That is, apparently, a bald-faced lie in the ReadFile API doc. First
implementation didn't use ReOpenFile, and it crashed all over the
place. ReOpenFile fixed it.
Stefan
To elaborate a little bit more, you can verify with a sample program
that ReadFile with OVERLAPPED does in fact modify the HANDLE's file
position. The documentation doesn't actually state one way or
another. My original attempt at a patch didn't have the ReOpenFile,
and we experienced regular read corruption. We scratched our heads
over it for a bit, and then hypothesized that someone must be mixing
read styles, which led to this ReOpenFile workaround, which
incidentally also solved the corruption problems. We wrote a similar
sample program to verify that when using ReOpenHandle, and changing
the file pointer of the duplicated handle, that the file pointer of
the original handle is not modified.
We did not actually try to identify the source of the mixed read
styles, but it seems like the only possible explanation.
On Thu, Feb 13, 2014 at 2:53 PM, Stefan Zager [off-list ref] wrote:
On Thu, Feb 13, 2014 at 2:51 PM, Karsten Blees [off-list ref] wrote:
quoted
Am 13.02.2014 19:38, schrieb Zachary Turner:
quoted
The only reason ReOpenFile is necessary at
all is because some code somewhere is mixing read-styles against the same
fd.
I don't understand...ReadFile with OVERLAPPED parameter doesn't modify the HANDLE's file position, so you should be able to mix read()/pread() however you like (as long as read() is only called from one thread).
That is, apparently, a bald-faced lie in the ReadFile API doc. First
implementation didn't use ReOpenFile, and it crashed all over the
place. ReOpenFile fixed it.
Stefan
To elaborate a little bit more, you can verify with a sample program
that ReadFile with OVERLAPPED does in fact modify the HANDLE's file
position. The documentation doesn't actually state one way or
another. My original attempt at a patch didn't have the ReOpenFile,
and we experienced regular read corruption. We scratched our heads
over it for a bit, and then hypothesized that someone must be mixing
read styles, which led to this ReOpenFile workaround, which
incidentally also solved the corruption problems. We wrote a similar
sample program to verify that when using ReOpenHandle, and changing
the file pointer of the duplicated handle, that the file pointer of
the original handle is not modified.
We did not actually try to identify the source of the mixed read
styles, but it seems like the only possible explanation.
On Thu, Feb 13, 2014 at 2:53 PM, Stefan Zager [off-list ref] wrote:
quoted
On Thu, Feb 13, 2014 at 2:51 PM, Karsten Blees [off-list ref] wrote:
quoted
Am 13.02.2014 19:38, schrieb Zachary Turner:
quoted
The only reason ReOpenFile is necessary at
all is because some code somewhere is mixing read-styles against the same
fd.
I don't understand...ReadFile with OVERLAPPED parameter doesn't modify the HANDLE's file position, so you should be able to mix read()/pread() however you like (as long as read() is only called from one thread).
That is, apparently, a bald-faced lie in the ReadFile API doc. First
implementation didn't use ReOpenFile, and it crashed all over the
place. ReOpenFile fixed it.
Stefan
Damn...you're right, multi-threaded git-index-pack works fine, but some tests fail badly. Mixed reads would have to be from git_mmap, which is the only other caller of pread().
A simple alternative to ReOpenHandle is to reset the file pointer to its original position, as in compat/pread.c::git_pread. Thus single-theaded code can mix read()/pread() at will, but multi-threaded code has to use pread() exclusively (which is usually the case anyway). A main thread using read() and background threads using pread() (which is technically allowed by POSIX) will fail with this solution.
This version passes the test suite on msysgit:
----8<----
ssize_t mingw_pread(int fd, void *buf, size_t count, off64_t offset)
{
DWORD bytes_read;
OVERLAPPED overlapped;
off64_t current;
memset(&overlapped, 0, sizeof(overlapped));
overlapped.Offset = (DWORD) offset;
overlapped.OffsetHigh = (DWORD) (offset >> 32);
current = lseek64(fd, 0, SEEK_CUR);
if (!ReadFile((HANDLE)_get_osfhandle(fd), buf, count, &bytes_read, &overlapped)) {
errno = err_win_to_posix(GetLastError());
return -1;
}
lseek64(fd, current, SEEK_SET);
return (ssize_t) bytes_read;
}
From: Stefan Zager <hidden> Date: 2016-06-15 22:59:55
On Fri, Feb 14, 2014 at 11:04 AM, Karsten Blees [off-list ref] wrote:
Damn...you're right, multi-threaded git-index-pack works fine, but some tests fail badly. Mixed reads would have to be from git_mmap, which is the only other caller of pread().
msysgit used git_mmap() as defined in compat/win32mmap.c, which does
not use pread.
Stefan
Hi,
I am planning to work on making pack access thread-safe as my GSoC
project, and after that, parallelize git blame or checkout. Or even use
the thread-safe pack access to improve the already parallel grep or
pack-objects.
With this in mind, I would like to know if the problem discussed in this
thread[1] is still an issue on the repos you folks work with (gentoo,
chromium, etc.). And also, could you please let me know which git
commands did you find to me more problematic in them, nowadays?
I downloaded chromium to give it a try and got (on a machine with i7 and
SSD, running Manjaro Linux):
- 17s on blame for a file with long history[2]
- 2m on blame for a huge file[3]
- 15s on log for both [2] and [3]
- 1s for git status
It seems quite a lot, especially with SSD, IMO.
[1] https://public-inbox.org/git/CA+TurHgyUK5sfCKrK+3xY8AeOg0t66vEvFxX=JiA9wXww7eZXQ@mail.gmail.com/
[2] ./chrome/browser/about_flags.cc (same with ./DEPS)
[3] third_party/sqlite/amalgamation/sqlite3.c (7.5M)
Best,
Matheus Tavares
On Tue, Apr 2, 2019 at 7:52 AM Matheus Tavares
[off-list ref] wrote:
I downloaded chromium to give it a try and got (on a machine with i7 and
SSD, running Manjaro Linux):
- 17s on blame for a file with long history[2]
- 2m on blame for a huge file[3]
- 15s on log for both [2] and [3]
- 1s for git status
It seems quite a lot, especially with SSD, IMO.
There have been a couple of optimizations that are probably still not
enabled by default because they only benefit large repos. So you may
want to check and turn them on before measuring anything:
commit-graph, pack bitmap, untracked cache or fsmonitor. All these
should be mentioned in 'git help config' (as starting point). Also
search "threads" in that man page because some commands may have multi
threads support but disabled by default for the same reason.
From your command list though, I think you might get the same results
(maybe with a bit faster 'git status') even with all optimizations on.
--
Duy
On Mon, Apr 1, 2019 at 10:07 PM Duy Nguyen [off-list ref] wrote:
On Tue, Apr 2, 2019 at 7:52 AM Matheus Tavares
[off-list ref] wrote:
quoted
I downloaded chromium to give it a try and got (on a machine with i7 and
SSD, running Manjaro Linux):
- 17s on blame for a file with long history[2]
- 2m on blame for a huge file[3]
- 15s on log for both [2] and [3]
- 1s for git status
It seems quite a lot, especially with SSD, IMO.
There have been a couple of optimizations that are probably still not
enabled by default because they only benefit large repos. So you may
want to check and turn them on before measuring anything:
commit-graph, pack bitmap, untracked cache or fsmonitor. All these
should be mentioned in 'git help config' (as starting point). Also
search "threads" in that man page because some commands may have multi
threads support but disabled by default for the same reason.
Nice, thanks for the suggestions!
From your command list though, I think you might get the same results
(maybe with a bit faster 'git status') even with all optimizations on.
Yes, you were right. With the optimizations on, I got the following
times on those same files:
- 17~18s on blame for about_flags.cc
- 1m50s~2m on blame for sqlite3.c
- 15s on log for both
- 0.3~0.5s on git status