git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH v12 00/13] libify apply and use lib in am, part 3
@ 2016-08-11 18:44 Christian Couder
  2016-08-11 18:44 ` [PATCH v12 01/13] builtin/apply: rename option parsing functions Christian Couder
                   ` (11 more replies)
  0 siblings, 12 replies; 15+ messages in thread
From: Christian Couder @ 2016-08-11 18:44 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason,
	Karsten Blees, Nguyen Thai Ngoc Duy, Stefan Beller, Eric Sunshine,
	Ramsay Jones, Johannes Sixt, René Scharfe, Stefan Naewe,
	Christian Couder

Goal
~~~~

This is a patch series about libifying `git apply` functionality, and
using this libified functionality in `git am`, so that no 'git apply'
process is spawn anymore. This makes `git am` significantly faster, so
`git rebase`, when it uses the am backend, is also significantly
faster.

Previous discussions and patches series
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This has initially been discussed in the following thread:

  http://thread.gmane.org/gmane.comp.version-control.git/287236/

Then the following patch series were sent:

RFC: http://thread.gmane.org/gmane.comp.version-control.git/288489/
v1: http://thread.gmane.org/gmane.comp.version-control.git/292324/
v2: http://thread.gmane.org/gmane.comp.version-control.git/294248/
v3: http://thread.gmane.org/gmane.comp.version-control.git/295429/
v4: http://thread.gmane.org/gmane.comp.version-control.git/296350/
v5: http://thread.gmane.org/gmane.comp.version-control.git/296490/
v6: http://thread.gmane.org/gmane.comp.version-control.git/297024/
v7: http://thread.gmane.org/gmane.comp.version-control.git/297193/
v8: https://public-inbox.org/git/20160627182429.31550-1-chriscool%40tuxfamily.org/
v9: https://public-inbox.org/git/20160730172509.22939-1-chriscool%40tuxfamily.org/
v10: https://public-inbox.org/git/20160808210337.5038-1-chriscool%40tuxfamily.org/
v11: https://public-inbox.org/git/20160811085229.19017-1-chriscool%40tuxfamily.org/

Highlevel view of the patches in the series
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This is "part 3" of the full patch series. I am resending only the
last 13 patches of the full series as "part 3", because I don't want
to resend the first 27 patches of v10 nearly as is. So "part 2" is
made of patch 01/40 from v11 and patches from 02/40 to 27/40 from v10.
The "part 1" is in "master" for some time now.

  - Patch 01/13 was v1, v2, v6, v7, v8, v9, v10 and v11.

Since v10 and v11 only the commit message has been changed. As
suggested by Stefan Naewe, 'api' as been replaced with 'API'.

  - Patches 02/13 to 04/13 were in v1, v2, v6, v7, v8, v9 and v10.

They haven't changed since v10.

Along with 01/13 they finish libifying the apply functionality that
was in builtin/apply.c and move it into apply.{c,h}, but the libified
functionality is not yet used in `git am`.

  - Patch 05/13 was in v6, v7, v8, v9 and v10, and hasn't changed.

It replaces some calls to error() with calls to error_errno().

  - Patches 06/13 to 10/13 were in v2, v6, v7, v8, v9 and v10.

They haven't changed since v10.

They implement a way to make the libified apply code silent by
changing the bool `apply_verbosely` into a tristate enum called
`apply_verbosity`, that can be one of `verbosity_verbose`,
`verbosity_normal` or `verbosity_silent`.

This is because "git am", since it was a shell script, has been
silencing the apply functionality by redirecting file descriptors to
/dev/null, but this is not acceptable in C.

  - Patch 11/13 was in v9 and v10, and hasn't changed.

It refactors `git apply` option parsing to make it possible for `git
am` to easily pass some command line options to the libified applied
code as suggested by Junio.

  - Patch 12/13 is new.

It replaces patch 33/40 in v10 (environment: add set_index_file())
that was a hack to make it possible for `git am` to use the libified
apply functionality on a different index file.

Now for this purpose, in this new patch, we add
"const char *index_file" into "struct apply_state", and when
"index_file" is set, we use hold_lock_file_for_update(), passing it
"state->index_file", instead of using hold_locked_index(), as it is
not possible to pass an index filename in hold_locked_index().

  - Patch 13/13 was in v1, v2, v6, v7, v8, v9 and v10.

This patch makes `git am` use the libified functionality. It now uses
"index_file" in "struct apply_state" added by 12/13 to pass the file
we want the index written into to the libified apply functionality.


General comments
~~~~~~~~~~~~~~~~

Now this patch series is shorter than previously. Hopefully also the
early part of this series until 05/13 or 11/13 will be ready soon to
be moved to next and master, and I may only need to resend the last 2
patches if anything.

I will send a diff between this version and v10 as a reply to this
email. (Note that in v11 only commit messages changed, so there is no
difference between v10 and v11.)

The benefits are not just related to not creating new processes. When
`git am` launched a `git apply` process, this new process had to read
the index from disk. Then after the `git apply`process had terminated,
`git am` dropped its index and read the index from disk to get the
index that had been modified by the `git apply`process. This was
inefficient and also prevented the split-index mechanism to provide
many performance benefits.

By the way, current work is ongoing to make it possible to use
split-index more easily by adding a config variable, see:

https://public-inbox.org/git/20160711172254.13439-1-chriscool%40tuxfamily.org/

Using an earlier version of this series as rebase material, Duy
explained split-index benefits along with this patch series like this:

 > Without the series, the picture is not so surprising. We run git-apply
 > 80+ times, each consists of this sequence
 >
 > read index
 > write index (cache tree updates only)
 > read index again
 > optionally initialize name hash (when new entries are added, I guess)
 > read packed-refs
 > write index
 >
 > With this series, we run a single git-apply which does
 >
 > read index (and sharedindex too if in split-index mode)
 > initialize name hash
 > write index 80+ times

(See: http://thread.gmane.org/gmane.comp.version-control.git/292324/focus=292460)

Links
~~~~~

This patch series is available here:

https://github.com/chriscool/git/commits/libify-apply-use-in-am

The previous versions are available there:

v1: https://github.com/chriscool/git/commits/libify-apply-use-in-am25 
v2: https://github.com/chriscool/git/commits/libify-apply-use-in-am54
v6: https://github.com/chriscool/git/commits/libify-apply-use-in-am65
v7: https://github.com/chriscool/git/commits/libify-apply-use-in-am75
v8: https://github.com/chriscool/git/commits/libify-apply-use-in-am97
v9: https://github.com/chriscool/git/commits/libify-apply-use-in-am106
v10: https://github.com/chriscool/git/commits/libify-apply-use-in-am114
v11: https://github.com/chriscool/git/commits/libify-apply-use-in-am116

Performance numbers
~~~~~~~~~~~~~~~~~~~

Numbers are only available for tests that have been performed on Linux
using a very early version of this series, though Johannes Sixt
reported great improvements on Windows. It could be interesting to get
detailed numbers on other platforms like Windows and OSX.

  - Around mid April Ævar did a huge many-hundred commit rebase on the
    kernel with untracked cache.

command: git rebase --onto 1993b17 52bef0c 29dde7c

Vanilla "next" without split index:                1m54.953s
Vanilla "next" with split index:                   1m22.476s
This series on top of "next" without split index:  1m12.034s
This series on top of "next" with split index:     0m15.678s

Ævar used his Debian laptop with SSD.

  - Around mid April I tested rebasing 13 commits in Booking.com's
    monorepo on a Red Hat 6.5 server with split-index and
    GIT_TRACE_PERFORMANCE=1.

With Git v2.8.0, the rebase took 6.375888383 s, with the git am
command launched by the rebase command taking 3.705677431 s.

With this series on top of next, the rebase took 3.044529494 s, with
the git am command launched by the rebase command taking 0.583521168
s.


Christian Couder (13):
  builtin/apply: rename option parsing functions
  apply: rename and move opt constants to apply.h
  Move libified code from builtin/apply.c to apply.{c,h}
  apply: make some parsing functions static again
  apply: use error_errno() where possible
  apply: make it possible to silently apply
  apply: don't print on stdout in verbosity_silent mode
  usage: add set_warn_routine()
  usage: add get_error_routine() and get_warn_routine()
  apply: change error_routine when silent
  apply: refactor `git apply` option parsing
  apply: learn to use a different index file
  builtin/am: use apply API in run_apply()

 apply.c           | 4861 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
 apply.h           |   34 +-
 builtin/am.c      |   65 +-
 builtin/apply.c   | 4814 +---------------------------------------------------
 git-compat-util.h |    3 +
 usage.c           |   15 +
 6 files changed, 4951 insertions(+), 4841 deletions(-)

-- 
2.9.2.769.gc0f0333


^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2016-08-26 23:46 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-11 18:44 [PATCH v12 00/13] libify apply and use lib in am, part 3 Christian Couder
2016-08-11 18:44 ` [PATCH v12 01/13] builtin/apply: rename option parsing functions Christian Couder
2016-08-11 18:44 ` [PATCH v12 02/13] apply: rename and move opt constants to apply.h Christian Couder
2016-08-11 18:44 ` [PATCH v12 04/13] apply: make some parsing functions static again Christian Couder
2016-08-11 18:44 ` [PATCH v12 05/13] apply: use error_errno() where possible Christian Couder
2016-08-11 18:44 ` [PATCH v12 06/13] apply: make it possible to silently apply Christian Couder
2016-08-11 18:44 ` [PATCH v12 07/13] apply: don't print on stdout in verbosity_silent mode Christian Couder
2016-08-11 18:44 ` [PATCH v12 08/13] usage: add set_warn_routine() Christian Couder
2016-08-11 18:44 ` [PATCH v12 09/13] usage: add get_error_routine() and get_warn_routine() Christian Couder
2016-08-11 18:44 ` [PATCH v12 10/13] apply: change error_routine when silent Christian Couder
2016-08-11 18:44 ` [PATCH v12 11/13] apply: refactor `git apply` option parsing Christian Couder
2016-08-11 18:45 ` [PATCH v12 12/13] apply: learn to use a different index file Christian Couder
2016-08-11 20:17   ` Junio C Hamano
2016-08-26 23:46     ` Christian Couder
2016-08-11 18:45 ` [PATCH v12 13/13] builtin/am: use apply API in run_apply() Christian Couder

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).