llvm dev - Oct 2021 - Proposal: introduce dependency on abseil when building benchmarks

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.
> 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).

> 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.

> 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?

> -Chris
>
>
>
> We want <https://github.com/google/benchmark/pull/1183> to add a
> dependency to abseil to the benchmarks project.
>
> Abseil has some requirements
>
<https://abseil.io/docs/cpp/platforms/platforms#:~:text=Abseil%20requires%20a%20code%20base,14%20through%20C%2B%2B20).>
> that may not perfectly match those of the impacted projects. For example,
> abseil stopped supporting Ubuntu 14.04 before its TLS.
>
> Naturally, projects snap to whichever version of benchmark they want to;
> but this new dependency would add an extra consideration when considering
> updating the version of benchmarks; and the need to handle the extra
> dependency (either by being OK with it being auto-downloaded, or via the
> other means described above)
>
> Are there any other issues that we're missing? Would anyone be hindered
by
> this adoption of abseil in google/benchmarks?
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20211009/8ac355be/attachment.html>

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
<mailto:clattner at nondot.org>> wrote:
>> On Sep 30, 2021, at 10:07 AM, Mircea Trofin via llvm-dev <llvm-dev
at lists.llvm.org <mailto: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/20211010/e22c3869/attachment.html>