llvm dev - Oct 2021 - Consolidating copies of google/benchmark in the repo (Was: Proposal: introduce dependency on abseil when building benchmarks)

Relabeling the thread, because we abandoned the abseil dependency in
google/benchmark upstream, but we still have the topic of 3 copies of
roughly the same thing in LLVM.

So the summary is: today we have:

llvm-test-suite/MicroBenchmarks/libs/benchmark/
llvm-project/llvm/utils/benchmark
llvm-project/libcxx/utils/google-benchmark

Each copy is maintained as needed, and cherrypicks are quite similar
across the copies.

If we want to get to one copy, Chris' proposal was to consolidate on
'llvm-project/llvm/utils/benchmark'; and presumably we don't have
any
reason to change the way we maintain the copy.

I'd like to do this consolidation. Is there any pushback to the plan above?

One thing that does come to mind, llvm-test-suite can be cloned separately
from llvm-project/llvm. We'd need to point to llvm (or just
llvm/utils/benchmark), and require that it be available on bots, for
example. Does that cause an issue for anyone?

Thanks,
Mircea

On Sun, Oct 10, 2021 at 9:08 PM Chris Lattner <clattner at nondot.org>
wrote:
> On Oct 9, 2021, at 7:32 PM, Mircea Trofin <mtrofin at google.com>
wrote:
>
> On Sat, Oct 9, 2021 at 6:15 PM Chris Lattner <clattner at nondot.org>
wrote:
>
>> On Sep 30, 2021, at 10:07 AM, Mircea Trofin via llvm-dev <
>> llvm-dev at lists.llvm.org> wrote:
>>
>> TL;DR; When either of LLVM_BUILD_BENCHMARKS or
LIBCXX_INCLUDE_BENCHMARKS
>> are enabled, as well as for llvm-test-suite, a dependency to abseil
would
>> either be auto-downloaded by the build system, or need to be
>> user-specifiable, or provided in the source tree.
>>
>>
>> Hi Mircea,
>>
>> As others have pointed out, this seems like a fairly problematic
>> dependency to take on.
>>
>> (For clarity, not attempting to argue pros/cons re abseil) it would be,
> if we wanted to start pulling more frequently and automatically from
> google/benchmark upstream. So far we've been cherry-picking changes,
from
> what I can tell.
>
>
> Ok, but why?  LLVM’s need for this should be fairly simple.  I don’t
> follow what mainline google/benchmark is trying to achieve, but do we need
> it?
>
> There are (afaik) 3 copies of the google/benchmark
>> <https://github.com/google/benchmark> project in the llvm tree:
in
>> llvm-test-suite, in llvm/utils, and in libcxx/utils/.
>>
>>
>> Ok, that sounds bad.  It seems like an intermediately good step is to
>> have a single copy of this in the monorepo (e.g.) in
llvm-project/utils,
>> and have livcxx and llvm-test-suite use that copy.  Is there any
downside
>> to consolidating these?
>>
>
> Would we also want to start more frequently sync-ing with the
> google/benchmark upstream - do we have a reason not to? (assuming
there's
> no abseil dep to worry about).
>
>
> I don’t understand the tradeoffs here - what do we get, vs what pain does
> it cause?  I don’t think that sync’ing with it is by itself a good thing,
> it should be motivated by cost/benefit analysis.
>
> I’m not up to speed on the goals of google/benchmark, but I suspect that
> google in general has different concerns than LLVM does.  It may be better
> to just straight-out fork or reimplement it from scratch.
>
> The benchmark code uses some functionality otherwise offered by abseil
>> <https://abseil.io/>. Over time, this is inconvenient: continued
need
>> for duplication for some features; integration issues in projects using
>> abseil due to macro conflicts; and overall bit rot / maintenance
overhead.
>>
>>
>> I’m not sure what you mean here - I think you are saying that there is
>> code from abseil that was copied into the google benchmark library, and
>> downstream code that uses both has issues?  Or are you saying it is
>> similar-but-different functionality that happens to use the same macro
>> names?
>>
>
> It's mainly the former. IIRC, we also hit an issue with the latter,
e.g.
> flag macros that are implemented slightly differently, but the root cause
> is  the former. *dominichamon@* may have more details, and there was
> another participant, *oontvoo@*, who expressed interest
>
<https://github.com/google/benchmark/pull/1183#issuecomment-889221305> in
> the abseil dependency, but haven't dug deeper into their motivation.
>
>
> Ok, it would be good to understand that better, because otherwise adding a
> dependency is strictly a negative IMO.
>
> Because I don’t understand the benefit, it seems like introducing a new
>> dependency is just a negative - can you explain the benefit more?
>>
>
> It would solve those downstream issues, but (and we'd have to check
with
> e.g. oontvoo@ if it addressed their scenario) we can definitely think of
> an alternative that addressed the concerns expressed here, and solved the
> original problem we had. (We basically jumped to abseil first because it
> seemed like the obvious thing, not realizing the fullness of the
> implications, and gathering the feedback here and also on the thread in
> google/benchmark makes me, at least, strongly believe we need to think of
> an alternative)
>
> I think at this point there are 2 topics: one is about how we consume
> google/benchmark in llvm; the other is the arguments against the abseil
> dep, which, even if for some reason llvm decided to freeze its copy of
> google/benchmark and thus not be affected, I think are sufficiently
> concerning to get us (switching hats to google/benchmark) rethink our
> approach.
>
> Should we focus this thread then on the former?
>
>
> That sounds like a great approach to me, figuring out the first part seems
> like a good prerequisite to the second part in any case.  Thank you for
> driving this discussion in an open and inclusive way!
>
> -Chris
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20211012/1556bcce/attachment.html>

Am Di., 12. Okt. 2021 um 15:20 Uhr schrieb Mircea Trofin <mtrofin at
google.com>:
> One thing that does come to mind, llvm-test-suite can be cloned separately
from llvm-project/llvm. We'd need to point to llvm (or just
llvm/utils/benchmark), and require that it be available on bots, for example.
Does that cause an issue for anyone?
I like to run the llvm-test-suite with other compilers as well such as
gcc. It requires LIT, but that can be installed separately
(https://pypi.org/project/lit/). I would already satisfied if there
there is a possibility to use alternative sources of google benchmark,
such as installed by the disto package manager (sudo apt install
libbenchmark1) or automatically downloaded using CMake:

include(FetchContent)
FetchContent_Declare(googlebenchmark
    GIT_REPOSITORY https://github.com/google/benchmark.git
)
FetchContent_MakeAvailable(googlebenchmark)

Michael

On Tue, 12 Oct 2021 at 21:20, Mircea Trofin <mtrofin at google.com> wrote:
> One thing that does come to mind, llvm-test-suite can be cloned separately
> from llvm-project/llvm. We'd need to point to llvm (or just
> llvm/utils/benchmark), and require that it be available on bots, for
> example. Does that cause an issue for anyone?
>
Because libc++ is now in the monorepo, unless it needs a different
(patched?) version from llvm's main one, it should not need its own copy.

AFAIK, the usual benchmark infrastructure compiles clang on demand and then
use that on the test-suite repo. However, that doesn't mean the repos are
on the same machine. For example, if you compile clang on a larger cluster,
install+pack and send to a more focued machine to do the
single-threaded-single-use-no-services benchmarking.

So, I'd say it should be safe to merge libc++ and llvm one, but not
necessarily the test-suite one.

If we don't have any local patches on those, we could also just clone it
from some known hash at runtime, or $deity forbid, as a submodule. :D

cheers,
--renato
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20211013/6cc4f442/attachment.html>