git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / Atom feed
From: Carlo Arenas <carenas@gmail.com>
To: "René Scharfe" <l.s.r@web.de>
Cc: git@vger.kernel.org, gitster@pobox.com,
	johannes.schindelin@gmx.de, avarab@gmail.com,
	michal.kiedrowicz@gmail.com
Subject: Re: [RFC PATCH v3 2/3] grep: make PCRE2 aware of custom allocator
Date: Wed, 7 Aug 2019 19:35:28 -0700
Message-ID: <CAPUEspj4CrEk6u4+8a5UBisxWsXcwOrOPQ5s9TktA6dZx5s+uQ@mail.gmail.com> (raw)
In-Reply-To: <c7f08e19-88a7-ca7f-90b9-54465e621d49@web.de>

On Wed, Aug 7, 2019 at 6:03 AM René Scharfe <l.s.r@web.de> wrote:
>
> Am 07.08.19 um 11:49 schrieb Carlo Arenas:
> > was hoping will perform better but it seems that testing can be done
> > only in windows
>
> nedmalloc works on other platforms as well.

I meant[1] it works reliably enough to be useful for performance testing.

goes without saying that the fact that I am using a virtualbox with 2
CPUs running Debian 10 on top of macOS (a macbook pro with 4 cores)
and the test uses by default 8 threads, doesn't help, but to share my
pain here is the result of running p7820 with my last reroll on top of
pu, comparing a build of the same code without NED (this tree) with
one with it (HEAD)

Test                                               this tree
HEAD
-------------------------------------------------------------------------------------------
7820.1: basic grep -i 'how.to'                     0.89(1.12+0.46)
0.95(1.23+0.49) +6.7%
7820.2: extended grep -i 'how.to'                  0.90(1.12+0.49)
0.92(1.19+0.46) +2.2%
7820.3: perl grep -i 'how.to'                      0.54(0.30+0.52)
0.53(0.39+0.52) -1.9%
7820.5: basic grep -i '^how to'                    0.89(1.13+0.47)
0.91(1.13+0.49) +2.2%
7820.6: extended grep -i '^how to'                 0.84(1.04+0.49)
0.94(1.27+0.47) +11.9%
7820.7: perl grep -i '^how to'                     0.49(0.34+0.47)
0.51(0.36+0.49) +4.1%
7820.9: basic grep -i '[how] to'                   1.51(2.31+0.51)
1.55(2.38+0.51) +2.6%
7820.10: extended grep -i '[how] to'               1.50(2.20+0.59)
1.56(2.30+0.62) +4.0%
7820.11: perl grep -i '[how] to'                   0.67(0.50+0.52)
0.62(0.50+0.55) -7.5%
7820.13: basic grep -i '\(e.t[^ ]*\|v.ry\) rare'   2.58(4.39+0.56)
2.64(4.45+0.60) +2.3%
7820.14: extended grep -i '(e.t[^ ]*|v.ry) rare'   2.60(4.41+0.56)
2.66(4.58+0.56) +2.3%
7820.15: perl grep -i '(e.t[^ ]*|v.ry) rare'       1.17(1.66+0.53)
1.23(1.84+0.45) +5.1%
7820.17: basic grep -i 'm\(ú\|u\)lt.b\(æ\|y\)te'   1.12(1.54+0.51)
1.14(1.70+0.44) +1.8%
7820.18: extended grep -i 'm(ú|u)lt.b(æ|y)te'      1.09(1.54+0.48)
1.14(1.62+0.49) +4.6%
7820.19: perl grep -i 'm(ú|u)lt.b(æ|y)te'          0.87(1.09+0.46)
0.90(1.20+0.43) +3.4%

and here one comparing two builds (both with NED)

Test                                               origin/pu
HEAD
-------------------------------------------------------------------------------------------
7820.1: basic grep -i 'how.to'                     1.00(1.24+0.55)
0.94(1.19+0.52) -6.0%
7820.2: extended grep -i 'how.to'                  0.90(1.15+0.49)
0.93(1.23+0.44) +3.3%
7820.3: perl grep -i 'how.to'                      0.52(0.37+0.51)
0.59(0.34+0.53) +13.5%
7820.5: basic grep -i '^how to'                    0.89(1.16+0.48)
0.90(1.17+0.47) +1.1%
7820.6: extended grep -i '^how to'                 0.92(1.17+0.50)
0.92(1.20+0.45) +0.0%
7820.7: perl grep -i '^how to'                     0.45(0.33+0.42)
0.54(0.29+0.57) +20.0%
7820.9: basic grep -i '[how] to'                   1.60(2.46+0.52)
1.61(2.39+0.62) +0.6%
7820.10: extended grep -i '[how] to'               1.71(2.67+0.56)
1.57(2.41+0.54) -8.2%
7820.11: perl grep -i '[how] to'                   0.66(0.61+0.51)
0.59(0.44+0.51) -10.6%
7820.13: basic grep -i '\(e.t[^ ]*\|v.ry\) rare'   2.69(4.49+0.66)
2.67(4.49+0.60) -0.7%
7820.14: extended grep -i '(e.t[^ ]*|v.ry) rare'   2.67(4.49+0.64)
2.64(4.54+0.54) -1.1%
7820.15: perl grep -i '(e.t[^ ]*|v.ry) rare'       1.23(1.80+0.47)
1.25(1.89+0.46) +1.6%
7820.17: basic grep -i 'm\(ú\|u\)lt.b\(æ\|y\)te'   1.13(1.64+0.47)
1.14(1.64+0.48) +0.9%
7820.18: extended grep -i 'm(ú|u)lt.b(æ|y)te'      1.16(1.68+0.46)
1.20(1.60+0.60) +3.4%
7820.19: perl grep -i 'm(ú|u)lt.b(æ|y)te'          0.90(1.16+0.48)
0.88(1.17+0.45) -2.2%

with the only relevant line (for my code) being 7820.19 where it would
seem it performs almost the same (eventhough just adding NED made it
initially worst)

note though that the fact there are 20% swings in parts of the code
that hasn't changed
or that where explicitly #ifdef out of my code changes doesn't give me
much confidence, but since the windows guys seem to be using NED by
default, I am hoping it works better there.

note also there were no segfaults (which is what was reported
originally) so something
else must be off.

> On Debian Testing with GCC
> 9.1.0 I need two changes to suppress some compiler warnings, though.
> Will post them as replies.
>
> "make USE_NED_ALLOCATOR=1 test" then reports these failures:
>
> t7816-grep-binary-pattern.sh                     (Wstat: 256 Tests: 145 Failed: 5)
>   Failed tests:  48, 54, 57, 60, 63
>   Non-zero exit status: 1

you got me so excited I dusted and old PC and was downloading the debian ISO
when I realized the error was not a segfault[2] but my bug.

sorry about it; I would swear I run the full test and it was clean,
but it was most likely
with PCRE1 or the system malloc, and definitely too late at night.

Thanks for your help so far, and while I know it is VERY ugly v4 at
least addresses that
(and uncovered a couple more bugs), thanks also Junio for rerolling it
into pu so at least
we have a chance for it to build and run, and hopefully eventually pass.

Carlo

[1] https://public-inbox.org/git/20190806085014.47776-3-carenas@gmail.com/
[2] https://dev.azure.com/git/git/_build/results?buildId=832

  parent reply	other threads:[~2019-08-08  2:35 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-05 11:51 [PATCH 0/1] Fix a problem with PCRE2 and nedmalloc, found via Azure Pipelines Johannes Schindelin via GitGitGadget
2019-08-05 11:51 ` [PATCH 1/1] pcre2: allow overriding the system allocator Johannes Schindelin via GitGitGadget
2019-08-05 16:19   ` Carlo Arenas
2019-08-05 16:27     ` Carlo Arenas
2019-08-05 19:32       ` Johannes Schindelin
2019-08-05 19:26     ` Johannes Schindelin
2019-08-05 21:53     ` Junio C Hamano
2019-08-06  6:24       ` Carlo Arenas
2019-08-06  8:50       ` [PATCH 0/3] grep: no leaks (WIP) Carlo Marcelo Arenas Belón
2019-08-06  8:50         ` [PATCH 1/3] grep: make PCRE1 aware of custom allocator Carlo Marcelo Arenas Belón
2019-08-08 13:54           ` Johannes Schindelin
2019-08-08 15:19             ` Carlo Arenas
2019-08-06  8:50         ` [PATCH 2/3] grep: make PCRE2 " Carlo Marcelo Arenas Belón
2019-08-08 13:56           ` Johannes Schindelin
2019-08-08 14:32             ` Carlo Arenas
2019-08-06  8:50         ` [PATCH 3/3] grep: avoid leak of chartables in PCRE2 Carlo Marcelo Arenas Belón
2019-08-06 16:36         ` [RFC PATCH v3 0/3] grep: no leaks or crashes (windows testing needed) Carlo Marcelo Arenas Belón
2019-08-06 16:36           ` [RFC PATCH v3 1/3] grep: make PCRE1 aware of custom allocator Carlo Marcelo Arenas Belón
2019-08-06 16:36           ` [RFC PATCH v3 2/3] grep: make PCRE2 " Carlo Marcelo Arenas Belón
2019-08-07  5:38             ` René Scharfe
2019-08-07  9:49               ` Carlo Arenas
2019-08-07 13:02                 ` René Scharfe
2019-08-07 13:08                   ` [PATCH 1/2] nedmalloc: do assignments only after the declaration section René Scharfe
2019-08-07 13:09                   ` [PATCH 2/2] nedmalloc: avoid compiler warning about unused value René Scharfe
2019-08-08  2:35                   ` Carlo Arenas [this message]
2019-08-08  7:07                     ` [RFC PATCH v3 2/3] grep: make PCRE2 aware of custom allocator René Scharfe
2019-08-08 12:38                       ` Carlo Arenas
2019-08-08 14:29                         ` René Scharfe
2019-08-08 20:18                           ` Johannes Schindelin
2019-08-07 18:15                 ` Junio C Hamano
2019-08-06 16:36           ` [RFC PATCH v3 3/3] grep: avoid leak of chartables in PCRE2 Carlo Marcelo Arenas Belón
2019-08-06 16:48           ` [RFC PATCH v3 0/3] grep: no leaks or crashes (windows testing needed) Junio C Hamano
2019-08-07 21:39           ` [RFC PATCH v4 " Carlo Marcelo Arenas Belón
2019-08-07 21:39             ` [RFC PATCH v4 1/3] grep: make PCRE1 aware of custom allocator Carlo Marcelo Arenas Belón
2019-08-07 21:39             ` [RFC PATCH v4 2/3] grep: make PCRE2 " Carlo Marcelo Arenas Belón
2019-08-07 22:28               ` Junio C Hamano
2019-08-07 21:39             ` [RFC PATCH v4 3/3] grep: avoid leak of chartables in PCRE2 Carlo Marcelo Arenas Belón
2019-08-09  3:02             ` [RFC PATCH v5 0/3] grep: almost no more leaks, hopefully no crashes Carlo Marcelo Arenas Belón
2019-08-09  3:02               ` [RFC PATCH v5 1/3] grep: make PCRE1 aware of custom allocator Carlo Marcelo Arenas Belón
2019-08-09  3:02               ` [RFC PATCH v5 2/3] grep: make PCRE2 " Carlo Marcelo Arenas Belón
2019-08-27  9:07                 ` Johannes Schindelin
2019-08-27 11:51                   ` Carlo Arenas
2019-10-03  5:02                     ` Junio C Hamano
2019-10-03  8:08                       ` Johannes Schindelin
2019-10-03 11:17                         ` Carlo Arenas
2019-10-03 18:23                           ` Johannes Schindelin
2019-10-03 22:57                           ` Junio C Hamano
2019-08-09  3:02               ` [RFC PATCH v5 3/3] grep: avoid leak of chartables in PCRE2 Carlo Marcelo Arenas Belón
2019-08-09 11:24               ` [RFC PATCH v5 0/3] grep: almost no more leaks, hopefully no crashes Carlo Arenas
2019-08-09 17:01                 ` René Scharfe
2019-08-09 17:46                   ` Junio C Hamano
2019-08-09 21:26                   ` Johannes Schindelin
2019-08-10  3:03                     ` [PATCH] SQUASH Carlo Marcelo Arenas Belón
2019-08-10  7:57                       ` René Scharfe
2019-08-10  8:42                         ` Carlo Arenas
2019-08-10 19:47                           ` René Scharfe
2019-08-12  7:35                             ` Carlo Arenas
2019-08-12 12:14                               ` René Scharfe
2019-08-12 12:28                                 ` Carlo Arenas
2019-08-10 13:57                       ` Johannes Schindelin
2019-08-10  3:05                     ` [RFC PATCH v5 0/3] grep: almost no more leaks, hopefully no crashes Carlo Arenas
2019-08-10  7:56                       ` René Scharfe
2019-08-10 12:40                         ` Carlo Arenas
2019-08-10 21:16                           ` René Scharfe
2019-08-08 20:21           ` [RFC PATCH v3 0/3] grep: no leaks or crashes (windows testing needed) Johannes Schindelin
2019-08-09  6:52             ` Carlo Arenas
2019-08-09 21:13               ` Johannes Schindelin

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=CAPUEspj4CrEk6u4+8a5UBisxWsXcwOrOPQ5s9TktA6dZx5s+uQ@mail.gmail.com \
    --to=carenas@gmail.com \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmx.de \
    --cc=l.s.r@web.de \
    --cc=michal.kiedrowicz@gmail.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

git@vger.kernel.org list mirror (unofficial, one of many)

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 git git/ https://public-inbox.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git
 note: .onion URLs require Tor: https://www.torproject.org/

code repositories for the project(s) associated with this inbox:

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

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git