All of lore.kernel.org
 help / color / mirror / Atom feed
From: Derrick Stolee <derrickstolee@github.com>
To: Junio C Hamano <gitster@pobox.com>,
	Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, git@jeffhostetler.com, me@ttaylorr.com,
	chakrabortyabhradeep79@gmail.com
Subject: Re: [PATCH] pack-bitmap: remove trace2 region from hot path
Date: Fri, 23 Sep 2022 14:07:46 -0400	[thread overview]
Message-ID: <edf25c5d-06f4-0b91-6773-f4fe7705c2f8@github.com> (raw)
In-Reply-To: <xmqq7d1uuh5f.fsf@gitster.g>

On 9/23/2022 1:36 PM, Junio C Hamano wrote:
> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>>     I noticed this while trying to backport the Abhradeep's lookup table
>>     work into GitHub's fork. Something went wrong in that process, causing
>>     this region to significantly slow down. It turns out that slow down does
>>     not reproduce on current 'master', which is good. I must have missed
>>     something while I was backporting.
>>     
>>     Regardless, the use of trace2_region_enter() here should be removed or
>>     replaced. For the sake of 2.38.0, this simple removal is safe enough.
> 
> Well, not doing this removal is even safer, if we know that it is
> not hurting in the -rc code.  It would be better than breaking our
> tests without knowing ;-)  I am not upset that the patch broke the
> test, by the way.  Compared to things silently breaking without
> surfacing, a loud breakage in tests is a much easier thing to
> handle.
> 
> My test run with the attached just finished, so that's what I am
> going to queue tentatively on top.

Whoops! I'm very sorry for not noticing that there was a test checking
for this trace. Your modification makes sense and I'll try to do a more
careful replacement of that test, if possible.

Thanks,
-Stolee

  reply	other threads:[~2022-09-23 18:09 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-23 13:00 [PATCH] pack-bitmap: remove trace2 region from hot path Derrick Stolee via GitGitGadget
2022-09-23 17:05 ` Junio C Hamano
2022-09-23 17:25   ` Junio C Hamano
2022-09-23 17:36 ` Junio C Hamano
2022-09-23 18:07   ` Derrick Stolee [this message]
2022-09-23 19:17   ` Taylor Blau
2022-09-26 13:17 ` [PATCH v2] " Derrick Stolee via GitGitGadget
2022-09-26 15:01   ` Ævar Arnfjörð Bjarmason
2022-09-26 17:31     ` Derrick Stolee
2022-09-27 18:37 ` [PATCH] " Jeff Hostetler

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=edf25c5d-06f4-0b91-6773-f4fe7705c2f8@github.com \
    --to=derrickstolee@github.com \
    --cc=chakrabortyabhradeep79@gmail.com \
    --cc=git@jeffhostetler.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=gitster@pobox.com \
    --cc=me@ttaylorr.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.