From: Junio C Hamano <hidden> Date: 2016-06-15 23:06:56
Junio C Hamano [off-list ref] writes:
During the discussion on the recent "git am" regression, I noticed
that the command reimplemented in C spawns one "mailsplit" and then
spawns "mailinfo" followed by "apply --index" to commit the changes
described in each message. As there are platforms where spawning
subprocess via run_command() interface is heavy-weight, something
that is conceptually very simple like "mailinfo" is better called
directly inside the process---something that is lightweight and
frequently used is where the overhead of run_command() would be felt
most.
Although I still haven't seen any offer to help from those who work
on the platforms that may benefit from this series the most, I have
some numbers on my desktop (Dell T3500 2.66GHz Xeon X5650 with 12GB,
running Ubuntu), where the cost of spawning is not as costly as
elsewhere, making this series less pressing.
Between 'master' and the version with this series (on 'jch'),
applying this 34-patch series itself on top of 'master' using "git
am", best of 5 numbers for running:
time git am mbox >/dev/null
are
(master) (with the series)
real 0m0.648s real 0m0.537s
user 0m0.358s user 0m0.338s
sys 0m0.172s sys 0m0.154s
I haven't re-read the series to catch leaks yet, so in that sense
the series is still not ready to be merged to 'next' (of course,
help with fresh set of eyes is very much appreciated here).
From: Stefan Beller <hidden> Date: 2016-06-15 23:06:56
On Tue, Oct 20, 2015 at 2:24 PM, Junio C Hamano [off-list ref] wrote:
Junio C Hamano [off-list ref] writes:
quoted
During the discussion on the recent "git am" regression, I noticed
that the command reimplemented in C spawns one "mailsplit" and then
spawns "mailinfo" followed by "apply --index" to commit the changes
described in each message. As there are platforms where spawning
subprocess via run_command() interface is heavy-weight, something
that is conceptually very simple like "mailinfo" is better called
directly inside the process---something that is lightweight and
frequently used is where the overhead of run_command() would be felt
most.
Although I still haven't seen any offer to help from those who work
on the platforms that may benefit from this series the most, I have
some numbers on my desktop (Dell T3500 2.66GHz Xeon X5650 with 12GB,
running Ubuntu), where the cost of spawning is not as costly as
elsewhere, making this series less pressing.
As far as I understand, this only helps for mailing list workflows, which
in my limited view of the world is only found in established infrastructure
projects, who tend to be maintained by people who run some kind of
ab-nomination of unix.
("Are there people in Windows land who heavily use the email workflow?")
Between 'master' and the version with this series (on 'jch'),
applying this 34-patch series itself on top of 'master' using "git
am", best of 5 numbers for running:
time git am mbox >/dev/null
are
(master) (with the series)
real 0m0.648s real 0m0.537s
user 0m0.358s user 0m0.338s
sys 0m0.172s sys 0m0.154s
I haven't re-read the series to catch leaks yet, so in that sense
the series is still not ready to be merged to 'next' (of course,
help with fresh set of eyes is very much appreciated here).
I'll take another look after sending out my series.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Ramsay Jones <hidden> Date: 2016-06-15 23:06:56
On 20/10/15 22:24, Junio C Hamano wrote:
Junio C Hamano [off-list ref] writes:
quoted
During the discussion on the recent "git am" regression, I noticed
that the command reimplemented in C spawns one "mailsplit" and then
spawns "mailinfo" followed by "apply --index" to commit the changes
described in each message. As there are platforms where spawning
subprocess via run_command() interface is heavy-weight, something
that is conceptually very simple like "mailinfo" is better called
directly inside the process---something that is lightweight and
frequently used is where the overhead of run_command() would be felt
most.
Although I still haven't seen any offer to help from those who work
on the platforms that may benefit from this series the most, I have
some numbers on my desktop (Dell T3500 2.66GHz Xeon X5650 with 12GB,
running Ubuntu), where the cost of spawning is not as costly as
elsewhere, making this series less pressing.
I suspect that I haven't tested exactly the same version as you, but I had
a quick look at testing this on Cygwin today. I have included a complete
transcript (below), so you can see what I did wrong! :-P
Between 'master' and the version with this series (on 'jch'),
applying this 34-patch series itself on top of 'master' using "git
am", best of 5 numbers for running:
time git am mbox >/dev/null
are
(master) (with the series)
real 0m0.648s real 0m0.537s
user 0m0.358s user 0m0.338s
sys 0m0.172s sys 0m0.154s
The corresponding times for me were:
(master) (with the series)
real 0m9.760s real 0m5.744s
user 0m0.531s user 0m0.656s
sys 0m5.726s sys 0m3.520s
So, yes, a noticeable improvement! :)
HTH
ATB,
Ramsay Jones
$ uname -a
CYGWIN_NT-6.3 satellite 2.2.1(0.289/5/3) 2015-08-20 11:42 x86_64 Cygwin
$ pwd
/home/ramsay/git
$ git log --decorate --oneline -1
74301d6 (HEAD -> master, origin/master, origin/HEAD) Sync with maint
$ ./git version
git version 2.6.2.280.g74301d6
$ git format-patch --stdout 2a5ce7c^..896df93 >mailinfo.mbox
$ git format-patch --stdout a4106a8^..559e247 >>mailinfo.mbox
$ git checkout -b master-mailinfo master
Switched to a new branch 'master-mailinfo'
$ time ./git am mailinfo.mbox
Applying: mailinfo: remove a no-op call convert_to_utf8(it, "")
Applying: mailinfo: fold decode_header_bq() into decode_header()
Applying: mailinfo: fix an off-by-one error in the boundary stack
Applying: mailinfo: explicitly close file handle to the patch output
Applying: mailinfo: move handle_boundary() lower
Applying: mailinfo: get rid of function-local static states
Applying: mailinfo: do not let handle_body() touch global "line" directly
Applying: mailinfo: do not let handle_boundary() touch global "line" directly
Applying: mailinfo: do not let find_boundary() touch global "line" directly
Applying: mailinfo: move global "line" into mailinfo() function
Applying: mailinfo: introduce "struct mailinfo" to hold globals
Applying: mailinfo: move keep_subject & keep_non_patch_bracket to struct mailinfo
Applying: mailinfo: move global "FILE *fin, *fout" to struct mailinfo
Applying: mailinfo: move filter/header stage to struct mailinfo
Applying: mailinfo: move patch_lines to struct mailinfo
Applying: mailinfo: move add_message_id and message_id to struct mailinfo
Applying: mailinfo: move use_scissors and use_inbody_headers to struct mailinfo
Applying: mailinfo: move metainfo_charset to struct mailinfo
Applying: mailinfo: move check for metainfo_charset to convert_to_utf8()
Applying: mailinfo: move transfer_encoding to struct mailinfo
Applying: mailinfo: move charset to struct mailinfo
Applying: mailinfo: move cmitmsg and patchfile to struct mailinfo
Applying: mailinfo: move [ps]_hdr_data to struct mailinfo
Applying: mailinfo: move content/content_top to struct mailinfo
Applying: mailinfo: handle_commit_msg() shouldn't be called after finding patchbreak
Applying: mailinfo: keep the parsed log message in a strbuf
Applying: mailinfo: move read_one_header_line() closer to its callers
Applying: mailinfo: move check_header() after the helpers it uses
Applying: mailinfo: move cleanup_space() before its users
Applying: mailinfo: move definition of MAX_HDR_PARSED closer to its use
Applying: mailinfo: libify
Applying: mailinfo: handle charset conversion errors in the caller
Applying: mailinfo: remove calls to exit() and die() deep in the callchain
Applying: am: make direct call to mailinfo
Applying: mailinfo: plug strbuf leak during continuation line handling
real 0m9.760s
user 0m0.531s
sys 0m5.726s
$
$ make clean >/dev/null 2>&1
$ make >out.mi 2>&1
$ ./git version
git version 2.6.2.315.g1e9f6ff
$ git describe
v2.6.2-315-g1e9f6ff
$ git checkout -b new-mailinfo master
Switched to a new branch 'new-mailinfo'
$ time ./git am mailinfo.mbox
Applying: mailinfo: remove a no-op call convert_to_utf8(it, "")
Applying: mailinfo: fold decode_header_bq() into decode_header()
Applying: mailinfo: fix an off-by-one error in the boundary stack
Applying: mailinfo: explicitly close file handle to the patch output
Applying: mailinfo: move handle_boundary() lower
Applying: mailinfo: get rid of function-local static states
Applying: mailinfo: do not let handle_body() touch global "line" directly
Applying: mailinfo: do not let handle_boundary() touch global "line" directly
Applying: mailinfo: do not let find_boundary() touch global "line" directly
Applying: mailinfo: move global "line" into mailinfo() function
Applying: mailinfo: introduce "struct mailinfo" to hold globals
Applying: mailinfo: move keep_subject & keep_non_patch_bracket to struct mailinfo
Applying: mailinfo: move global "FILE *fin, *fout" to struct mailinfo
Applying: mailinfo: move filter/header stage to struct mailinfo
Applying: mailinfo: move patch_lines to struct mailinfo
Applying: mailinfo: move add_message_id and message_id to struct mailinfo
Applying: mailinfo: move use_scissors and use_inbody_headers to struct mailinfo
Applying: mailinfo: move metainfo_charset to struct mailinfo
Applying: mailinfo: move check for metainfo_charset to convert_to_utf8()
Applying: mailinfo: move transfer_encoding to struct mailinfo
Applying: mailinfo: move charset to struct mailinfo
Applying: mailinfo: move cmitmsg and patchfile to struct mailinfo
Applying: mailinfo: move [ps]_hdr_data to struct mailinfo
Applying: mailinfo: move content/content_top to struct mailinfo
Applying: mailinfo: handle_commit_msg() shouldn't be called after finding patchbreak
Applying: mailinfo: keep the parsed log message in a strbuf
Applying: mailinfo: move read_one_header_line() closer to its callers
Applying: mailinfo: move check_header() after the helpers it uses
Applying: mailinfo: move cleanup_space() before its users
Applying: mailinfo: move definition of MAX_HDR_PARSED closer to its use
Applying: mailinfo: libify
Applying: mailinfo: handle charset conversion errors in the caller
Applying: mailinfo: remove calls to exit() and die() deep in the callchain
Applying: am: make direct call to mailinfo
Applying: mailinfo: plug strbuf leak during continuation line handling
real 0m5.744s
user 0m0.656s
sys 0m3.520s
$
$ git rev-parse master-mailinfo^{tree} new-mailinfo^{tree}
cebb1a110f3af9f4393bb66b942ae28e2743f233
cebb1a110f3af9f4393bb66b942ae28e2743f233
$
From: Johannes Sixt <hidden> Date: 2016-06-15 23:07:01
Am 21.10.2015 um 17:51 schrieb Ramsay Jones:
On 20/10/15 22:24, Junio C Hamano wrote:
quoted
Junio C Hamano [off-list ref] writes:
some numbers on my desktop (Dell T3500 2.66GHz Xeon X5650 with 12GB,
running Ubuntu),
I suspect that I haven't tested exactly the same version as you, but I had
a quick look at testing this on Cygwin today. I have included a complete
transcript (below), so you can see what I did wrong! :-P
quoted
Between 'master' and the version with this series (on 'jch'),
applying this 34-patch series itself on top of 'master' using "git
am", best of 5 numbers for running:
time git am mbox >/dev/null
are
(master) (with the series)
real 0m0.648s real 0m0.537s
user 0m0.358s user 0m0.338s
sys 0m0.172s sys 0m0.154s
The corresponding times for me were:
(master) (with the series)
real 0m9.760s real 0m5.744s
user 0m0.531s user 0m0.656s
sys 0m5.726s sys 0m3.520s
So, yes, a noticeable improvement! :)
Same here, on Windows built with the old msysgit environment:
(master) (with the series)
real 0m3.147s real 0m1.947s
user 0m0.016s user 0m0.000s
sys 0m0.015s sys 0m0.031s
Although I tested 'git am patches/*' where the patches/* are the result
of 'git-format-patch v2.6.1..github/jc/mailinfo-lib'.
-- Hannes
From: Johannes Schindelin <hidden> Date: 2016-06-15 23:07:03
Hi,
On Wed, 21 Oct 2015, Johannes Sixt wrote:
Am 21.10.2015 um 17:51 schrieb Ramsay Jones:
quoted
On 20/10/15 22:24, Junio C Hamano wrote:
quoted
Junio C Hamano [off-list ref] writes:
some numbers on my desktop (Dell T3500 2.66GHz Xeon X5650 with 12GB,
running Ubuntu),
I suspect that I haven't tested exactly the same version as you, but I had
a quick look at testing this on Cygwin today. I have included a complete
transcript (below), so you can see what I did wrong! :-P
quoted
Between 'master' and the version with this series (on 'jch'),
applying this 34-patch series itself on top of 'master' using "git
am", best of 5 numbers for running:
time git am mbox >/dev/null
are
(master) (with the series)
real 0m0.648s real 0m0.537s
user 0m0.358s user 0m0.338s
sys 0m0.172s sys 0m0.154s
The corresponding times for me were:
(master) (with the series)
real 0m9.760s real 0m5.744s
user 0m0.531s user 0m0.656s
sys 0m5.726s sys 0m3.520s
So, yes, a noticeable improvement! :)
Same here, on Windows built with the old msysgit environment:
(master) (with the series)
real 0m3.147s real 0m1.947s
user 0m0.016s user 0m0.000s
sys 0m0.015s sys 0m0.031s
Although I tested 'git am patches/*' where the patches/* are the result of
'git-format-patch v2.6.1..github/jc/mailinfo-lib'.