Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: Lynne <dev@lynne.ee>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH 4/7] checkasm: use pointers for start/stop functions
Date: Mon, 17 Jul 2023 19:48:40 +0200 (CEST)
Message-ID: <N_ZxzXY--3-9@lynne.ee> (raw)
In-Reply-To: <2404688.o4v6flLIFt@basile.remlab.net>

Jul 17, 2023, 07:18 by remi@remlab.net:

> Le sunnuntaina 16. heinäkuuta 2023, 23.32.21 EEST Lynne a écrit :
>
>> Introducing additional overhead in the form of a dereference is a point
>> where instability can creep in. Can you guarantee that a context will
>> always remain in L1D cache,
>>
>
> L1D is not involved here. In version 2, the pointers are cached locally.
>
>> as opposed to just reading the raw CPU timing
>> directly where that's supported.
>>
>
> Of course not. Raw CPU timing is subject to noise from interrupts (and 
> whatever those interrupts trigger). And that's not just theoretical. I've 
> experienced it and it sucks. Raw CPU timing is much noisier than Linux perf.
>
> And because it has also been proven vastly insecure, it's been disabled on Arm 
> for a long time, and is being disabled on RISC-V too now.
>
>> > But I still argue that that is, either way, completely negligible compared
>> > to the *existing* overhead. Each loop is making 4 system calls, and each
>> > of those system call requires a direct call (to PLT) and an indirect
>> > branch (from GOT). If you have a problem with the two additional function
>> > calls, then you can't be using Linux perf in the first place.
>>
>> You don't want to ever use linux perf in the first place, it's second class.
>>
>
> No it isn't. The interface is more involved than just reading a CSR; and sure 
> I'd prefer the simple interface that RDCYCLE is all other things being equal. 
> But other things are not equal. Linux perf is in fact *more* accurate by 
> virtue of not *wrongly* counting other things. And it does not threaten the 
> security of the entire system, so it will work inside a rented VM or an 
> unprivileged process.
>

Threaten? This is a development tool first and foremost.
If anyone doesn't want to use rdcycle, they can use linux perf, it still works,
with or without the patch.


>> I don't think it's worth changing the direct inlining we had before. You're
>> not interested in whether or not the same exact code is ran between
>> platforms,
>>
>
> Err, I am definitely interested in doing exactly that. I don't want to have to 
> reconfigure and recompile the entire FFmpeg just to switch between Linux perf 
> and raw cycle counter. A contrario, I *do* want to compare performance between 
> vendors once the hardware is available.
>

That's a weak reason to compromise the accuracy of a development tool.


>> just that the code that's measuring timing is as efficient and
>> low overhead as possible.
>>
>
> Of course not. Low overhead is irrelevant here. The measurement overhead is 
> know and is subtracted. What we need is stable/reproducible overhead, and 
> accurate measurements.
>

Which is what TSC or the equivalent gets you. It's noisy, but that's because
it's better and higher accuracy than having to roundtrip through the kernel.


> And that's assuming the stuff works at all. You can argue that we should use 
> Arm PMU and RISC-V RDCYCLE, and that Linux perf sucks, all you want. PMU 
> access will just throw a SIGILL and end the checkasm process with zero 
> measurements. The rest of the industry wants to use system calls for informed 
> reasons. I don't think you, or even the whole FFmpeg project, can win that 
> argument against OS and CPU vendors.
>

Either way, I don't agree with this patch, not accepting it.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

  reply	other threads:[~2023-07-17 17:48 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-14 18:26 [FFmpeg-devel] [PATCH 0/7] checkasm RISC-V Linux perf enablement Rémi Denis-Courmont
2023-07-14 18:28 ` [FFmpeg-devel] [PATCH 1/7] checkasm: fix Linux perf cleanup Rémi Denis-Courmont
2023-07-14 18:28 ` [FFmpeg-devel] [PATCH 2/7] checkasm: improve Linux perf error message Rémi Denis-Courmont
2023-07-14 18:28 ` [FFmpeg-devel] [PATCH 3/7] checkasm: make perf macros functional Rémi Denis-Courmont
2023-07-14 18:28 ` [FFmpeg-devel] [PATCH 4/7] checkasm: use pointers for start/stop functions Rémi Denis-Courmont
2023-07-15  8:05   ` Lynne
2023-07-15  8:25     ` Rémi Denis-Courmont
2023-07-15 17:43       ` Lynne
2023-07-15 20:13         ` Rémi Denis-Courmont
2023-07-16 20:32           ` Lynne
2023-07-17  5:18             ` Rémi Denis-Courmont
2023-07-17 17:48               ` Lynne [this message]
2023-07-17 18:09                 ` Rémi Denis-Courmont
2023-07-18 21:32                   ` Lynne
2023-07-19 15:58                     ` Rémi Denis-Courmont
2023-07-24 21:26                   ` [FFmpeg-devel] [TC] " Martin Storsjö
2023-07-24 21:33                     ` Nicolas George
2023-07-24 22:19                     ` Lynne
2023-07-24 22:57                       ` Kieran Kunhya
2023-07-25  6:44                       ` Martin Storsjö
2023-07-14 18:28 ` [FFmpeg-devel] [PATCH 5/7] checkasm: remove unused variable Rémi Denis-Courmont
2023-07-14 18:28 ` [FFmpeg-devel] [PATCH 6/7] checkasm: allow run-time fallback to AV_READ_TIME Rémi Denis-Courmont
2023-07-14 18:28 ` [FFmpeg-devel] [PATCH 7/7] configure: enable Linux perf on RISC-V by default Rémi Denis-Courmont

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=N_ZxzXY--3-9@lynne.ee \
    --to=dev@lynne.ee \
    --cc=ffmpeg-devel@ffmpeg.org \
    /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 Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


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