[llvm-dev] Proposal: introduce dependency on abseil when building benchmarks

[Mircea Trofin via llvm-dev]

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.

Details

There are (afaik) 3 copies of the google/benchmark project in the llvm tree: in llvm-test-suite, in llvm/utils, and in libcxx/utils/.

The benchmark code uses some functionality otherwise offered by abseil. 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.

We want to add a dependency to abseil to the benchmarks project.

Abseil has some requirements 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?

Thanks,

Mircea.

[Mircea Trofin via llvm-dev]

Gentle reminder - I'd plan on moving forward with the abseil dependency by the EOW, unless there's pushback.

Thanks!

[Reid Kleckner via llvm-dev]

On Thu, Sep 30, 2021 at 10:08 AM Mircea Trofin via llvm-dev <llvm...@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.

Could you please elaborate on which of these approaches will be used for LLVM? How will this affect regular LLVM developers?

----

This is truly unrelated, but I have a lot of feelings about this, and I will use this opportunity to inappropriately complain that the benchmarks library has been spamming me with cmake warnings about std::regex for years: https://bugs.llvm.org/show_bug.cgi?id=38874 The CMake step really ought to be warning-clean.

[Mircea Trofin via llvm-dev]

On Tue, Oct 5, 2021 at 8:57 PM Reid Kleckner <r...@google.com> wrote:

On Thu, Sep 30, 2021 at 10:08 AM Mircea Trofin via llvm-dev <llvm...@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.

Could you please elaborate on which of these approaches will be used for LLVM? How will this affect regular LLVM developers?

Assuming regular LLVM developers means developers that don't enable LLVM_BUILD_BENCHMARKS, nor LIBCXX_INCLUDE_BENCHMARKS, then they are not affected.

The current PR in "benchmarks" upstream is set up so that it will either download the abseil dependency at build time, or, if the location of abseil is specified via a cmake flag, then it uses that one (which covers the last 2 options). I don't know if the first option is acceptable by those that enable LLVM_BUILD_BENCHMARKS / LIBCXX_INCLUDE_BENCHMARKS, and this is something I'm hoping to discover with this thread. If it is acceptable, it's a transparent option, so it won't directly impact those users either.

----

This is truly unrelated, but I have a lot of feelings about this, and I will use this opportunity to inappropriately complain that the benchmarks library has been spamming me with cmake warnings about std::regex for years: https://bugs.llvm.org/show_bug.cgi?id=38874 The CMake step really ought to be warning-clean.

Ack. Added an issue on the project side: https://github.com/google/benchmark/issues/1236 (maybe it has better visibility)

[Mehdi AMINI via llvm-dev]

On Tue, Oct 5, 2021 at 9:26 PM Mircea Trofin via llvm-dev <llvm...@lists.llvm.org> wrote:

On Tue, Oct 5, 2021 at 8:57 PM Reid Kleckner <r...@google.com> wrote:

On Thu, Sep 30, 2021 at 10:08 AM Mircea Trofin via llvm-dev <llvm...@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.

Could you please elaborate on which of these approaches will be used for LLVM? How will this affect regular LLVM developers?

Assuming regular LLVM developers means developers that don't enable LLVM_BUILD_BENCHMARKS, nor LIBCXX_INCLUDE_BENCHMARKS, then they are not affected.

The current PR in "benchmarks" upstream is set up so that it will either download the abseil dependency at build time, or, if the location of abseil is specified via a cmake flag, then it uses that one (which covers the last 2 options). I don't know if the first option is acceptable by those that enable LLVM_BUILD_BENCHMARKS / LIBCXX_INCLUDE_BENCHMARKS, and this is something I'm hoping to discover with this thread. If it is acceptable, it's a transparent option, so it won't directly impact those users either.

What should we expect in terms of ability to benchmark LLVM on various platforms / OSes? It seems like we will tie ourselves to Abseil, and I don't know anything about how their compatibility matrix compares to LLVM one? (and if it will continue being true in the foreseeable future?)

Thanks,

-- 

Mehdi

 

----

This is truly unrelated, but I have a lot of feelings about this, and I will use this opportunity to inappropriately complain that the benchmarks library has been spamming me with cmake warnings about std::regex for years: https://bugs.llvm.org/show_bug.cgi?id=38874 The CMake step really ought to be warning-clean.

Ack. Added an issue on the project side: https://github.com/google/benchmark/issues/1236 (maybe it has better visibility)

_______________________________________________
LLVM Developers mailing list
llvm...@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

[Mircea Trofin via llvm-dev]

On Tue, Oct 5, 2021 at 9:37 PM Mehdi AMINI <joke...@gmail.com> wrote:

On Tue, Oct 5, 2021 at 9:26 PM Mircea Trofin via llvm-dev <llvm...@lists.llvm.org> wrote:

On Tue, Oct 5, 2021 at 8:57 PM Reid Kleckner <r...@google.com> wrote:

On Thu, Sep 30, 2021 at 10:08 AM Mircea Trofin via llvm-dev <llvm...@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.

Could you please elaborate on which of these approaches will be used for LLVM? How will this affect regular LLVM developers?

Assuming regular LLVM developers means developers that don't enable LLVM_BUILD_BENCHMARKS, nor LIBCXX_INCLUDE_BENCHMARKS, then they are not affected.

The current PR in "benchmarks" upstream is set up so that it will either download the abseil dependency at build time, or, if the location of abseil is specified via a cmake flag, then it uses that one (which covers the last 2 options). I don't know if the first option is acceptable by those that enable LLVM_BUILD_BENCHMARKS / LIBCXX_INCLUDE_BENCHMARKS, and this is something I'm hoping to discover with this thread. If it is acceptable, it's a transparent option, so it won't directly impact those users either.

What should we expect in terms of ability to benchmark LLVM on various platforms / OSes? It seems like we will tie ourselves to Abseil, and I don't know anything about how their compatibility matrix compares to LLVM one? (and if it will continue being true in the foreseeable future?)

The goal isn't for benchmark to change its dependency matrix due to the new dependency ( +dominic hamon , please correct me). So from the perspective of LLVM, this doesn't change anything. Granted, there is now a strictly higher probability for breaking changes, if abseil decides to change its support matrix and break benchmark, and thus affect the latter's LLVM users.

[Mircea Trofin via llvm-dev]

On Thu, Oct 7, 2021 at 8:10 AM dominic hamon <dom...@google.com> wrote:

On Thu, Oct 7, 2021 at 4:06 PM Mircea Trofin <mtr...@google.com> wrote:

On Tue, Oct 5, 2021 at 9:37 PM Mehdi AMINI <joke...@gmail.com> wrote:

On Tue, Oct 5, 2021 at 9:26 PM Mircea Trofin via llvm-dev <llvm...@lists.llvm.org> wrote:

On Tue, Oct 5, 2021 at 8:57 PM Reid Kleckner <r...@google.com> wrote:

On Thu, Sep 30, 2021 at 10:08 AM Mircea Trofin via llvm-dev <llvm...@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.

Could you please elaborate on which of these approaches will be used for LLVM? How will this affect regular LLVM developers?

Assuming regular LLVM developers means developers that don't enable LLVM_BUILD_BENCHMARKS, nor LIBCXX_INCLUDE_BENCHMARKS, then they are not affected.

The current PR in "benchmarks" upstream is set up so that it will either download the abseil dependency at build time, or, if the location of abseil is specified via a cmake flag, then it uses that one (which covers the last 2 options). I don't know if the first option is acceptable by those that enable LLVM_BUILD_BENCHMARKS / LIBCXX_INCLUDE_BENCHMARKS, and this is something I'm hoping to discover with this thread. If it is acceptable, it's a transparent option, so it won't directly impact those users either.

What should we expect in terms of ability to benchmark LLVM on various platforms / OSes? It seems like we will tie ourselves to Abseil, and I don't know anything about how their compatibility matrix compares to LLVM one? (and if it will continue being true in the foreseeable future?)

The goal isn't for benchmark to change its dependency matrix due to the new dependency ( +dominic hamon , please correct me). So from the perspective of LLVM, this doesn't change anything. Granted, there is now a strictly higher probability for breaking changes, if abseil decides to change its support matrix and break benchmark, and thus affect the latter's LLVM users.

We're reducing our dependency matrix (dropping support for older compilers/OSs) as part of the adoption of abseil. This may impact llvm if there was an expectation to continue to benchmark against those older compilers/OSs.

Isn't that support drop part of usual business, though (i.e. drop after TLS)?

https://google.github.io/benchmark/dependencies.html is the current policy but with abseil that "best effort" support will no longer be viable.

[Renato Golin via llvm-dev]

On Thu, 30 Sept 2021 at 18:08, Mircea Trofin via llvm-dev <llvm...@lists.llvm.org> wrote:

Abseil has some requirements that may not perfectly match those of the impacted projects. For example, abseil stopped supporting Ubuntu 14.04 before its TLS. 

The list of supported platforms is *definitely* too small for LLVM users. Half of their support is "best effort", which really isn't going to cut it once we forcefully depend on it.

We definitely run benchmarks on X86_64, Arm32/64, MIPS, PowerPC (Linux, Mac and Windows on a mix of those), and there are probably people running on SystemZ, RISCV and other less known architectures.

What is your plan for all the other platforms where abseil isn't supported?

cheers,

--renato

[Renato Golin via llvm-dev]

On Tue, 5 Oct 2021 at 16:07, Mircea Trofin via llvm-dev <llvm...@lists.llvm.org> wrote:

Gentle reminder - I'd plan on moving forward with the abseil dependency by the EOW, unless there's pushback.

Also, please, never assume consensus if no one replies. Lack of push back isn't the same as agreement.

You proposed a breaking change, gave 7 days for everyone to find this email, and assumed EOW would be fair game if no one replied.

This is really not good enough.

[Mircea Trofin via llvm-dev]

What would be the preferred duration?

 

[dominic hamon via llvm-dev]

On Thu, Oct 7, 2021 at 4:06 PM Mircea Trofin <mtr...@google.com> wrote:

On Tue, Oct 5, 2021 at 9:37 PM Mehdi AMINI <joke...@gmail.com> wrote:

On Tue, Oct 5, 2021 at 9:26 PM Mircea Trofin via llvm-dev <llvm...@lists.llvm.org> wrote:

On Tue, Oct 5, 2021 at 8:57 PM Reid Kleckner <r...@google.com> wrote:

On Thu, Sep 30, 2021 at 10:08 AM Mircea Trofin via llvm-dev <llvm...@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.

Could you please elaborate on which of these approaches will be used for LLVM? How will this affect regular LLVM developers?

Assuming regular LLVM developers means developers that don't enable LLVM_BUILD_BENCHMARKS, nor LIBCXX_INCLUDE_BENCHMARKS, then they are not affected.

The current PR in "benchmarks" upstream is set up so that it will either download the abseil dependency at build time, or, if the location of abseil is specified via a cmake flag, then it uses that one (which covers the last 2 options). I don't know if the first option is acceptable by those that enable LLVM_BUILD_BENCHMARKS / LIBCXX_INCLUDE_BENCHMARKS, and this is something I'm hoping to discover with this thread. If it is acceptable, it's a transparent option, so it won't directly impact those users either.

What should we expect in terms of ability to benchmark LLVM on various platforms / OSes? It seems like we will tie ourselves to Abseil, and I don't know anything about how their compatibility matrix compares to LLVM one? (and if it will continue being true in the foreseeable future?)

The goal isn't for benchmark to change its dependency matrix due to the new dependency ( +dominic hamon , please correct me). So from the perspective of LLVM, this doesn't change anything. Granted, there is now a strictly higher probability for breaking changes, if abseil decides to change its support matrix and break benchmark, and thus affect the latter's LLVM users.

We're reducing our dependency matrix (dropping support for older compilers/OSs) as part of the adoption of abseil. This may impact llvm if there was an expectation to continue to benchmark against those older compilers/OSs.

https://google.github.io/benchmark/dependencies.html is the current policy but with abseil that "best effort" support will no longer be viable.

 

[dominic hamon via llvm-dev]

On Thu, Oct 7, 2021 at 4:12 PM Mircea Trofin <mtr...@google.com> wrote:

On Thu, Oct 7, 2021 at 8:10 AM dominic hamon <dom...@google.com> wrote:

On Thu, Oct 7, 2021 at 4:06 PM Mircea Trofin <mtr...@google.com> wrote:

On Tue, Oct 5, 2021 at 9:37 PM Mehdi AMINI <joke...@gmail.com> wrote:

On Tue, Oct 5, 2021 at 9:26 PM Mircea Trofin via llvm-dev <llvm...@lists.llvm.org> wrote:

On Tue, Oct 5, 2021 at 8:57 PM Reid Kleckner <r...@google.com> wrote:

On Thu, Sep 30, 2021 at 10:08 AM Mircea Trofin via llvm-dev <llvm...@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.

Could you please elaborate on which of these approaches will be used for LLVM? How will this affect regular LLVM developers?

Assuming regular LLVM developers means developers that don't enable LLVM_BUILD_BENCHMARKS, nor LIBCXX_INCLUDE_BENCHMARKS, then they are not affected.

The current PR in "benchmarks" upstream is set up so that it will either download the abseil dependency at build time, or, if the location of abseil is specified via a cmake flag, then it uses that one (which covers the last 2 options). I don't know if the first option is acceptable by those that enable LLVM_BUILD_BENCHMARKS / LIBCXX_INCLUDE_BENCHMARKS, and this is something I'm hoping to discover with this thread. If it is acceptable, it's a transparent option, so it won't directly impact those users either.

What should we expect in terms of ability to benchmark LLVM on various platforms / OSes? It seems like we will tie ourselves to Abseil, and I don't know anything about how their compatibility matrix compares to LLVM one? (and if it will continue being true in the foreseeable future?)

The goal isn't for benchmark to change its dependency matrix due to the new dependency ( +dominic hamon , please correct me). So from the perspective of LLVM, this doesn't change anything. Granted, there is now a strictly higher probability for breaking changes, if abseil decides to change its support matrix and break benchmark, and thus affect the latter's LLVM users.

We're reducing our dependency matrix (dropping support for older compilers/OSs) as part of the adoption of abseil. This may impact llvm if there was an expectation to continue to benchmark against those older compilers/OSs.

Isn't that support drop part of usual business, though (i.e. drop after TLS)?

yes, except for the best effort bit below.

[Renato Golin via llvm-dev]

On Thu, 7 Oct 2021 at 16:46, Mircea Trofin <mtr...@google.com> wrote:

What would be the preferred duration?

It's not at all about duration, it's about making sure the right people have looked at the proposal and agreed with the plan.

Unless people that actually build and run benchmarks have agreed with your proposal, you should not merge a clear breaking change.

Of course, you can always merge, and break people's stuff, and revert, and then discuss, but I'd strongly encourage you not to do that, as it isn't nice to other people.

You should ask around, who are the people who build benchmarks. Check with the target owners, buildbot owners, past threads on benchmarking, and make sure they're all included in the discussion.

It's really easy to miss an email like this and it's just out of luck that I didn't.

For now, given the extremely limited platform support abseil provides, even considering the best effort part, I'd strongly discourage you to merge.

[Mircea Trofin via llvm-dev]

On Thu, Oct 7, 2021 at 8:56 AM Renato Golin <reng...@gmail.com> wrote:

On Thu, 7 Oct 2021 at 16:46, Mircea Trofin <mtr...@google.com> wrote:

What would be the preferred duration?

It's not at all about duration, it's about making sure the right people have looked at the proposal and agreed with the plan.

Unless people that actually build and run benchmarks have agreed with your proposal, you should not merge a clear breaking change.

One of the goals of this thread was identifying who those folks may be.

 

Of course, you can always merge, and break people's stuff, and revert, and then discuss, but I'd strongly encourage you not to do that, as it isn't nice to other people.

You should ask around, who are the people who build benchmarks. Check with the target owners, buildbot owners, past threads on benchmarking, and make sure they're all included in the discussion.

Yup, that's the purpose of this thread. 

It's really easy to miss an email like this and it's just out of luck that I didn't.

Is there a more appropriate channel of communication where owners could be identified?

[Renato Golin via llvm-dev]

On Thu, 7 Oct 2021 at 17:01, Mircea Trofin <mtr...@google.com> wrote:

Unless people that actually build and run benchmarks have agreed with your proposal, you should not merge a clear breaking change.

One of the goals of this thread was identifying who those folks may be.

And yet, you propose to follow through if no one objected. Which is never a good idea for breaking changes.

It's really easy to miss an email like this and it's just out of luck that I didn't.

Is there a more appropriate channel of communication where owners could be identified?

No, this is the right place.

But you either wait for people to find this thread (however long it takes), or you actively search for them (as I outlined before) and include them in the conversation, for example, CC'ing them in the thread.

[Mircea Trofin via llvm-dev]

On Thu, Oct 7, 2021 at 9:12 AM Renato Golin <reng...@gmail.com> wrote:

On Thu, 7 Oct 2021 at 17:01, Mircea Trofin <mtr...@google.com> wrote:

Unless people that actually build and run benchmarks have agreed with your proposal, you should not merge a clear breaking change.

One of the goals of this thread was identifying who those folks may be.

And yet, you propose to follow through if no one objected. Which is never a good idea for breaking changes.

To be clear, the fact they are breaking changes is what we're trying to determine. It was my intention to spur attention to the thread (given the silence), and providing a timeline can help.

 

It's really easy to miss an email like this and it's just out of luck that I didn't.

Is there a more appropriate channel of communication where owners could be identified?

No, this is the right place.

But you either wait for people to find this thread (however long it takes), or you actively search for them (as I outlined before) and include them in the conversation, for example, CC'ing them in the thread.

Yup, but they need to be identified first (i.e. a bit of a catch 22 if no one replies)

[David Blaikie via llvm-dev]

There's probably no better mailing list, etc. It's an unfortunate situation that active developers don't/can't keep up with llvm-dev (it's a lot, even without commits lists, etc) such that it generally isn't sufficient to email a non-targeted email to llvm-dev and expect to find the interested parties in some particular subsystem/feature area/etc. Generally if you don't already know at least a representative subset of interested parties you may need to go looking for them similar to finding appropriate code reviewers - check commit histories in the code in question, commit histories in buildbot configurations that might be using this feature (in this case), and maybe then buildbot owners that enable the configuration. If you can find at least a couple of interested parties in such a search and nothing else turns up that may be adequate to provide a variety of perspectives/sufficient consensus to make a decision and/or they might be able to point to others they know of who are working in that area.

[Mircea Trofin via llvm-dev]

On Thu, Oct 7, 2021 at 8:40 AM Renato Golin <reng...@gmail.com> wrote:

On Thu, 30 Sept 2021 at 18:08, Mircea Trofin via llvm-dev <llvm...@lists.llvm.org> wrote:

Abseil has some requirements that may not perfectly match those of the impacted projects. For example, abseil stopped supporting Ubuntu 14.04 before its TLS. 

The list of supported platforms is *definitely* too small for LLVM users. Half of their support is "best effort", which really isn't going to cut it once we forcefully depend on it.

We definitely run benchmarks on X86_64, Arm32/64, MIPS, PowerPC (Linux, Mac and Windows on a mix of those), and there are probably people running on SystemZ, RISCV and other less known architectures.

Making sure we're talking about the same thing: this is strictly about benchmarks that require flipping LLVM_BUILD_BENCHMARKS or LIBCXX_INCLUDE_BENCHMARKS; and it's about exploring how those specific benchmarks relate to the 'benchmark' project which we currently have as a source copy in the llvm tree (i.e. we're sometimes manually cherry-picking changes over, but we're not updating it through any automated means)

I looked under llvm-zorg, and can't find any bot configuration that flips those flags in the first place, so perhaps these specific benchmarks are run elsewhere (or my search was naive - I just grep-ed for '_BENCHMARKS' - and only got one entry turning off TEST_SUITE_RUN_BENCHMARKS, which appears to be unrelated)?

From the OS x CPU matrix above, it seems the main gap is in the CPU column - since abseil supports Linux, MacOS, and Windows, but doesn't appear to specifically support mips, powerpc, systemz, or riscv. So if anyone runs these kinds of benchmarks (i.e. the ones that require flipping those cmake flags) on those CPUs, *and* we wanted, in llvm, to periodically auto-update the source copy of 'benchmark' project, only then we would have a breaking change.

I think there are 2 topics:

- I think the main one is: was there ever any intent to automatically update the 2 copies of 'benchmarks' in the llvm tree? what about the one in llvm-test-suite? 

- the second is getting a better understanding of where folks build and run those specific benchmarks (it'd help the upstream 'benchmark' project with data points). 

Renato, would you mind detailing your scenario (i.e. which of those 2 types of benchmarks, on which OSxCPU)

If there are other folks that can chime in with their scenario, it'd be much appreciated.

Thanks!

[Tom Stellard via llvm-dev]

On 9/30/21 10:07 AM, Mircea Trofin via llvm-dev 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.
>
> Details
>

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

Does this mean it would no longer be possible to build the test-suite
on any architecture besides x86_64, aarch64, and arm?

-Tom

> 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?
>
> Thanks,
> Mircea.
>
>

[Renato Golin via llvm-dev]

Ok, it looks like we're talking past each other, so I'll be extra clear. This is not meant to be rude, just making sure we're in the same page.

On Thu, 7 Oct 2021 at 17:20, Mircea Trofin <mtr...@google.com> wrote:

And yet, you propose to follow through if no one objected. Which is never a good idea for breaking changes.

To be clear, the fact they are breaking changes is what we're trying to determine. It was my intention to spur attention to the thread (given the silence), and providing a timeline can help.

There are many ways to drive attention to a thread that hasn't received the due attention, for example:

* "Politely ping" the thread, by saying "hey, has anyone seen this, I need some feedback" or such.

* Reply to the thread including more people and making it clear that you're doing so by saying: "Adding more people to broaden the search for reply".

* Chase other channels, like IRC, Disco{rd|urse}, other tools that you know are people that can help.

Your "gentle reminder" isn't good because it assumed(*) that:

* everyone that should have read your email did read the email

* your change is obviously-good &| non-breaking

* silence with consensus

None of which is true.

(*) Even if you personally didn't assume, the reply itself had the consequence (merge EOW) as if it was true.

Yup, but they need to be identified first (i.e. a bit of a catch 22 if no one replies)

Both LLVM's mailing list and IRC channels are high-traffic, meaning it's really easy to get lost in the noise. If you don't get a reply in a few days, it's very likely that no one saw it.

People also work no different time zones, have different week schedules, holidays, sickness, etc. Some people have alerts for emails, others read a digest at the end of the day. Some people work on the project full time, others on their spare time.

Most people, however, are more responsive if you copy/@ them directly than if you just post a generic thread somewhere. But how to find those people?

First, old timers know more people than new contributors, so asking them is a starting point. 

But most of them will say similar things:

* Look for the code owners, buildbot maintainers

* Search the mailing list, phabricator or bugzilla for past topics and copy the people involved

* Use a different communication channel (ex. IRC) to ask around

Once enough people are involved, they themselves know who else to copy, and the process becomes a lot easier.

Now, where to look for people?

* Code owners are listed in the file: llvm-project/llvm/CODE_OWNERS.txt

* Infrastructure maintainers are listed in: llvm-project/llvm/RELEASE_TESTERS.TXT and https://lab.llvm.org/buildbot/#/workers (among others)

* Mailing list and bugzilla searches aren't very effective, but help spot a few people.

Once you get enough people, just copy them on the initial RFC, and it will have a much higher probability of reaching the right people.

cheers,

--renato

[Mircea Trofin via llvm-dev]

On Thu, Oct 7, 2021 at 9:47 AM Tom Stellard <tste...@redhat.com> wrote:

On 9/30/21 10:07 AM, Mircea Trofin via llvm-dev 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.
>
> Details
>
> 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/.
>
> 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.
>
> 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.
>

Does this mean it would no longer be  possible to build the test-suite
on any architecture besides x86_64, aarch64, and arm?

*if* there's a desire to keep in sync automatically with the upstream benchmark suite (which we don't currently do)

[Mircea Trofin via llvm-dev]

Thanks!

[Tom Stellard via llvm-dev]

On 10/7/21 9:49 AM, Mircea Trofin wrote:

>
>
> On Thu, Oct 7, 2021 at 9:47 AM Tom Stellard <tste...@redhat.com <mailto:tste...@redhat.com>> wrote:
>
> On 9/30/21 10:07 AM, Mircea Trofin via llvm-dev 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.
> >
> > Details
> >

> > There are (afaik) 3 copies of the google/benchmark <https://github.com/google/benchmark <https://github.com/google/benchmark>> project in the llvm tree: in llvm-test-suite, in llvm/utils, and in libcxx/utils/.
> >
> > The benchmark code uses some functionality otherwise offered by abseil <https://abseil.io/ <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.
> >
> > We want <https://github.com/google/benchmark/pull/1183 <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) <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.

> >
>
> Does this mean it would no longer be  possible to build the test-suite
> on any architecture besides x86_64, aarch64, and arm?
>
> *if* there's a desire to keep in sync automatically with the upstream benchmark suite (which we don't currently do)
>

Ok, where in the test-suite is this code?

-Tom

>
>
> -Tom
>
> > 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?
> >
> > Thanks,
> > Mircea.
> >
> >
> > _______________________________________________
> > LLVM Developers mailing list

> > llvm...@lists.llvm.org <mailto:llvm...@lists.llvm.org>
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev <https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev>

[Renato Golin via llvm-dev]

On Thu, 7 Oct 2021 at 17:46, Mircea Trofin <mtr...@google.com> wrote:

Making sure we're talking about the same thing: this is strictly about benchmarks that require flipping LLVM_BUILD_BENCHMARKS or LIBCXX_INCLUDE_BENCHMARKS; and it's about exploring how those specific benchmarks relate to the 'benchmark' project which we currently have as a source copy in the llvm tree (i.e. we're sometimes manually cherry-picking changes over, but we're not updating it through any automated means)

Honestly, I didn't understand what this means... :)

But I mostly run the benchmarks via CMake from a recently built LLVM copy, and I don't do that "professionally", so I'm not the right person to such things.

I looked under llvm-zorg, and can't find any bot configuration that flips those flags in the first place, so perhaps these specific benchmarks are run elsewhere (or my search was naive - I just grep-ed for '_BENCHMARKS' - and only got one entry turning off TEST_SUITE_RUN_BENCHMARKS, which appears to be unrelated)?

AFAIK, most of the benchmarks using our test-suite are not done via buildbots. I've set up one in the long past but even then it didn't seem to be the norm.

Back then, I remember every single major target was running the test-suite in benchmark mode in an internal infrastructure, because they were also running private benchmarks and had their own local reporting system that allowed them to track regressions.

Looking for bot-owners to add to the thread only gets you as far as knowing who they think should be copied, or if you're luck, they're the same people. :)

From the OS x CPU matrix above, it seems the main gap is in the CPU column - since abseil supports Linux, MacOS, and Windows, but doesn't appear to specifically support mips, powerpc, systemz, or riscv. So if anyone runs these kinds of benchmarks (i.e. the ones that require flipping those cmake flags) on those CPUs, *and* we wanted, in llvm, to periodically auto-update the source copy of 'benchmark' project, only then we would have a breaking change.

There's also the fact that abseil only supports "Linux with Arm64" and "ChromeOS on Armv7", so it's not really a matrix, just a list of combinations.

- I think the main one is: was there ever any intent to automatically update the 2 copies of 'benchmarks' in the llvm tree? what about the one in llvm-test-suite? 

Right, and only the people who actually run benchmarks can answer that.

- the second is getting a better understanding of where folks build and run those specific benchmarks (it'd help the upstream 'benchmark' project with data points).

Yes, unfortunately, because many of those are internal, people don't communicate what they do too well. And if some of the additional benchmarks are secret, they may not even be able to communicate.

Renato, would you mind detailing your scenario (i.e. which of those 2 types of benchmarks, on which OSxCPU)

I'm afraid I cannot, as I haven't done any benchmarking for a long time. Hopefully Tom may be able to help you more than I can.

cheers,

--renato

[Mircea Trofin via llvm-dev]

On Thu, Oct 7, 2021 at 9:58 AM Tom Stellard <tste...@redhat.com> wrote:

On 10/7/21 9:49 AM, Mircea Trofin wrote:
>
>
> On Thu, Oct 7, 2021 at 9:47 AM Tom Stellard <tste...@redhat.com <mailto:tste...@redhat.com>> wrote:
>
>     On 9/30/21 10:07 AM, Mircea Trofin via llvm-dev 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.
>      >
>      > Details
>      >
>      > There are (afaik) 3 copies of the google/benchmark <https://github.com/google/benchmark <https://github.com/google/benchmark>> project in the llvm tree: in llvm-test-suite, in llvm/utils, and in libcxx/utils/.
>      >
>      > The benchmark code uses some functionality otherwise offered by abseil <https://abseil.io/ <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.
>      >
>      > We want <https://github.com/google/benchmark/pull/1183 <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) <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.
>      >
>
>     Does this mean it would no longer be  possible to build the test-suite
>     on any architecture besides x86_64, aarch64, and arm?
>
> *if* there's a desire to keep in sync automatically with the upstream benchmark suite (which we don't currently do)
>

Ok, where in the test-suite is this code?

llvm-test-suite/MicroBenchmarks/libs/benchmark/

for completeness, the other 2 copies* pertinent to the llvm project overall are:

llvm-project/llvm/utils/benchmark
llvm-project/libcxx/utils/google-benchmark

 

*AFAIK

[Mircea Trofin via llvm-dev]

On Thu, Oct 7, 2021 at 10:00 AM Renato Golin <reng...@gmail.com> wrote:

On Thu, 7 Oct 2021 at 17:46, Mircea Trofin <mtr...@google.com> wrote:

Making sure we're talking about the same thing: this is strictly about benchmarks that require flipping LLVM_BUILD_BENCHMARKS or LIBCXX_INCLUDE_BENCHMARKS; and it's about exploring how those specific benchmarks relate to the 'benchmark' project which we currently have as a source copy in the llvm tree (i.e. we're sometimes manually cherry-picking changes over, but we're not updating it through any automated means)

Honestly, I didn't understand what this means... :)

But I mostly run the benchmarks via CMake from a recently built LLVM copy, and I don't do that "professionally", so I'm not the right person to such things.

Oh, OK - so what's your cmake  invocation like - i.e. if you don't flip those 2 flags, then you probably don't build those benchmarks.

Another way to put it: do you build anything under:

llvm-test-suite/MicroBenchmarks/libs/benchmark/

llvm/utils/benchmark
libcxx/utils/google-benchmark

[Renato Golin via llvm-dev]

On Thu, 7 Oct 2021 at 18:06, Mircea Trofin <mtr...@google.com> wrote:

Another way to put it: do you build anything under:

llvm-test-suite/MicroBenchmarks/libs/benchmark/

llvm/utils/benchmark
libcxx/utils/google-benchmark

I don't, not intentionally. but others might.

[Michael Kruse via llvm-dev]

Am Do., 7. Okt. 2021 um 11:47 Uhr schrieb Mircea Trofin via llvm-dev
<llvm...@lists.llvm.org>:

> I looked under llvm-zorg, and can't find any bot configuration that flips those flags in the first place, so perhaps these specific benchmarks are run elsewhere (or my search was naive - I just grep-ed for '_BENCHMARKS' - and only got one entry turning off TEST_SUITE_RUN_BENCHMARKS, which appears to be unrelated)?

Google Benchmark (from MicroBenchmarks/libs/benchmark) automatically
compiles/runs if TES_SUITE_SUBDIRS includes MicroBenchmarks, which is
included by default.

TEST_SUITE_RUN_BENCHMARKS is unrelated

Michael

_______________________________________________
LLVM Developers mailing list
llvm...@lists.llvm.org

https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

[Mircea Trofin via llvm-dev]

On Thu, Oct 7, 2021 at 10:34 AM Michael Kruse <llv...@meinersbur.de> wrote:

Am Do., 7. Okt. 2021 um 11:47 Uhr schrieb Mircea Trofin via llvm-dev
<llvm...@lists.llvm.org>:
> I looked under llvm-zorg, and can't find any bot configuration that flips those flags in the first place, so perhaps these specific benchmarks are run elsewhere (or my search was naive - I just grep-ed for '_BENCHMARKS' - and only got one entry turning off TEST_SUITE_RUN_BENCHMARKS, which appears to be unrelated)?

Google Benchmark (from MicroBenchmarks/libs/benchmark) automatically
compiles/runs if TES_SUITE_SUBDIRS includes MicroBenchmarks, which is
included by default.

This is about bots building llvm-test-suite, correct?

TEST_SUITE_RUN_BENCHMARKS is unrelated

Michael

[James Y Knight via llvm-dev]

On Thu, Oct 7, 2021 at 11:40 AM Renato Golin via llvm-dev <llvm...@lists.llvm.org> wrote:

On Thu, 30 Sept 2021 at 18:08, Mircea Trofin via llvm-dev <llvm...@lists.llvm.org> wrote:

Abseil has some requirements that may not perfectly match those of the impacted projects. For example, abseil stopped supporting Ubuntu 14.04 before its TLS. 

The list of supported platforms is *definitely* too small for LLVM users. Half of their support is "best effort", which really isn't going to cut it once we forcefully depend on it.

We definitely run benchmarks on X86_64, Arm32/64, MIPS, PowerPC (Linux, Mac and Windows on a mix of those), and there are probably people running on SystemZ, RISCV and other less known architectures.

I think this isn't actually as big a deal as it sounds. The primary difference between "supported" and "best-effort" is whether they have working CI for that platform. Furthermore, many unlisted platforms do work. In practice, abseil has a history of accepting patches to add portability for "unsupported" platforms.

E.g. to take two examples:

https://github.com/abseil/abseil-cpp/pull/1021 adding support for AIX

https://github.com/abseil/abseil-cpp/pull/904 adding support for Linux on HPPA, Alpha, IA64, and SH.

 

What is your plan for all the other platforms where abseil isn't supported?

The plan should simply be: submit patches if it's broken on an architecture or OS that someone cares about.

Old versions of the toolchains is a different matter, however -- Abseil's general promise is to support an old release of a C++ compiler for 5 years after it has been superseded by a newer version. And supporting old compilers tends to be a significant burden, so unlike porting to a new CPU or OS, that isn't simply a "patches welcome" situation -- there would have to be a compelling case made that preserving support for an compiler older than that was important enough to be worth the hassle.

See also: https://abseil.io/blog/20201001-platforms

[Mircea Trofin via llvm-dev]

(Just to make sure this aspect isn't lost) all of this matters *if* we want to start updating the copies of `benchmark` in llvm / llvm-test-suite from upstream `benchmark`. It's all moot if we are OK to continue, indefinitely, with maintaining our own copies. (to echo Renato - this topic of how we want to consume `benchmark` is a question for the folks that care about running those benchmarks. I'll try find & add more to to/cc in a bit)

[Renato Golin via llvm-dev]

On Thu, 7 Oct 2021 at 20:51, James Y Knight <jykn...@google.com> wrote:

The plan should simply be: submit patches if it's broken on an architecture or OS that someone cares about.

This only works if the people who currently run those benchmarks in LLVM can continue running them _before_ submitting patches.

Old versions of the toolchains is a different matter, however -- Abseil's general promise is to support an old release of a C++ compiler for 5 years after it has been superseded by a newer version. And supporting old compilers tends to be a significant burden, so unlike porting to a new CPU or OS, that isn't simply a "patches welcome" situation -- there would have to be a compelling case made that preserving support for an compiler older than that was important enough to be worth the hassle.

Because it supports clang, it should be fine. 

[Mircea Trofin via llvm-dev]

+more folks based on recent updates to either of the 3 `google/benchmark` (`benchmark` for short) copies in LLVM. Apologies if I missed anyone.

The summary is: in the `benchmark` repo we're considering adopting abseil. For LLVM, if we're OK with continuing to maintain our own fork(s) of the `benchmark` project and opportunistically patch them as needed, the abseil dependency upstream should have no real impact[*].

That dependency does impact us in LLVM, should we want to have a more automated way for depending on `benchmark`, such as periodic pulls. The abseil support guarantees may not cover sufficiently well the scenarios in which folks build and run these kinds of benchmarks. This thread has more details.

====

[*]Short of patches upstream that are so heavily dependent on abseil that porting them to the forks in llvm would prove super-challenging. Abseil would primarily used at the periphery of the project, e.g flags, for string manipulation, and concurrency, so probability of such challenging changes is low (but I'm biased).

[Mircea Trofin via llvm-dev]

<Fixed an email address I copied incorrectly>

[Matthias Braun via llvm-dev]

> On Sep 30, 2021, at 10:07 AM, Mircea Trofin via llvm-dev <llvm...@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.

FWIW: I'm not a fan of auto-downloading stuff. That's just a sneaky to add a dependency that sure may not give trouble to the users where the auto-download succeeds. But many companies have their build farms isolated from the internet and security people would not be happy if we just download a blob of code from a separate project that can change somewhat unnoticed by users of LLVM.

Can't we copy the thing into the LLVM repository (aka vendoring) like we copied the benchmark library? I feel that things become a different story when we actually add dependencies...

- Matthias

[Mircea Trofin via llvm-dev]

On Thu, Oct 7, 2021 at 6:57 PM Matthias Braun <matt...@fb.com> wrote:

> On Sep 30, 2021, at 10:07 AM, Mircea Trofin via llvm-dev <llvm...@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.

FWIW: I'm not a fan of auto-downloading stuff. That's just a sneaky to add a dependency that sure may not give trouble to the users where the auto-download succeeds. But many companies have their build farms isolated from the internet and security people would not be happy if we just download a blob of code from a separate project that can change somewhat unnoticed by users of LLVM.

Can't we copy the thing into the LLVM repository (aka vendoring) like we copied the benchmark library? I feel that things become a different story when we actually add dependencies...

That is one of the options available in the proposed change in the benchmark project upstream (the 'provided in the source tree' option). So if we decided, in LLVM, that we wanted to update the forks we have of the benchmark project (assuming the other issues mentioned on this thread were resolved), then it's up to us to set it up like you described. 

- Matthias

[Stella Laurenzo via llvm-dev]

Perhaps an unpopular opinion, but I don't see the value proposition in foundation things like benchmark taking such convenience dependencies. Abseil is not a lightweight dependency, either from the code or the build system perspective, and it will limit portability (such a thing is hard to prove, but I can speak from experience, being a downstream that has taken the time to scalpel Abseil out in order to achieve better compatibility and footprint).

In my experience, there are typically a small handful of things that actually boost such a project (usually related to strings and collections). For foundation components, I would far rather see such things forked in if needed vs adding a hard dependency. Even better if components like abseil were unobtrusive in keeping things separated for this form of sharing.

My project will most likely fork benchmark or switch to something lighter if it goes down this path. LLVM can of course take its own path but I generally consider forking when I see foundation components start down the path of losing control of their dependencies.

[Mircea Trofin via llvm-dev]

This is valuable feedback, and I think it would help if you could please post it in on the thread on the benchmark project side (i.e. https://github.com/google/benchmark/pull/1183), since it would help the decision making process there - thanks!

[Stella Laurenzo via llvm-dev]

Let me parse that thread and see if I have something to offer. I am well aware of the internal Google forces referenced and at play in causing these kinds of changes -- and they are pretty ingrained in the way that Google builds its software. And I suspect that pushing against them is likely not productive. I just think that OSS communities need to think carefully about the hidden costs of riding such coattails.

Again, in the difficult to prove category -- I used to be an abseil proponent and may still be for software at certain levels of the value chain. But as a low level component targeting weird configurations, our life got substantially better when we ditched it, and it was not cheap to do so, since the conveniences it provides tend to wend their way through from a style perspective. I would summarize the issues as it being very apparent that Abseil has a small compatibility footprint that the maintainers are paying attention to, there is little control/care (that I could find) on internal sprawl or footprint, and it felt like we were always fighting a militant build system that was always unhappy about something. Some of this feedback was given but incentives were clearly not aligned (which is fine -- there were no hard feelings, just pragmatism) and we didn't judge that there would be. So we disconnected ourselves.

I don't know what the right answer is for LLVM -- this may be innocent enough (or at least scoped enough that we can back out and take a different tack if it causes issues). I'm just cautious of such things, having been burned before.

[Chris Lattner via llvm-dev]

On Sep 30, 2021, at 10:07 AM, Mircea Trofin via llvm-dev <llvm...@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.

There are (afaik) 3 copies of the 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?

The benchmark code uses some functionality otherwise offered by abseil. 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?

Because I don’t understand the benefit, it seems like introducing a new dependency is just a negative - can you explain the benefit more?

-Chris

We want to add a dependency to abseil to the benchmarks project.

Abseil has some requirements that may not perfectly match those of the impacted projects. For example, abseil stopped supporting Ubuntu 14.04 before its TLS. 

[Mircea Trofin via llvm-dev]

On Sat, Oct 9, 2021 at 6:15 PM Chris Lattner <clat...@nondot.org> wrote:

On Sep 30, 2021, at 10:07 AM, Mircea Trofin via llvm-dev <llvm...@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 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. 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 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 Lattner via llvm-dev]

On Oct 9, 2021, at 7:32 PM, Mircea Trofin <mtr...@google.com> wrote:

On Sat, Oct 9, 2021 at 6:15 PM Chris Lattner <clat...@nondot.org> wrote:

On Sep 30, 2021, at 10:07 AM, Mircea Trofin via llvm-dev <llvm...@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 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. 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 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

[Mircea Trofin via llvm-dev]

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

[Michael Kruse via llvm-dev]

Am Di., 12. Okt. 2021 um 15:20 Uhr schrieb Mircea Trofin <mtr...@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

[Mircea Trofin via llvm-dev]

On Tue, Oct 12, 2021 at 2:13 PM Michael Kruse <llv...@meinersbur.de> wrote:

Am Di., 12. Okt. 2021 um 15:20 Uhr schrieb Mircea Trofin <mtr...@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)

We can probably have a set of steps we try on the llvm-test-suite to find the dependency, like:

1 one of: user-specified `benchmark` dir,  user-specified llvm dir, or user specified directive to 'check installed' or to 'fetch latest'

2 try a heuristic: either ../llvm/utils/benchmark, or, if that's not there, try fetching it, and if this fails, ok, can't build

Probably looking at ../llvm would 'just work' on the build bots, too? (meaning, without needing to change buildbot scripts)

 

Michael

[Michael Kruse via llvm-dev]

Am Di., 12. Okt. 2021 um 17:09 Uhr schrieb Mircea Trofin <mtr...@google.com>:
> We can probably have a set of steps we try on the llvm-test-suite to find the dependency, like:
> 1 one of: user-specified `benchmark` dir, user-specified llvm dir, or user specified directive to 'check installed' or to 'fetch latest'

That sounds fine.

> 2 try a heuristic: either ../llvm/utils/benchmark, or, if that's not there, try fetching it, and if this fails, ok, can't build
> Probably looking at ../llvm would 'just work' on the build bots, too? (meaning, without needing to change buildbot scripts)

No, sources are at different directories relative to the build
directory. Typically, this is '../llvm.src/llvm' [1,2], but I am not
sure for all builders, but others might be different [3]. Builds by
LNT might be more complicated [4].

Generally, I also don't really like hardcoding paths, makes
reproducing problems more difficult.

[1] https://github.com/llvm/llvm-zorg/blob/529cab62b53092cafe42c0a127480bede2806206/zorg/buildbot/builders/PollyBuilder.py#L23
[2] https://github.com/llvm/llvm-zorg/blob/529cab62b53092cafe42c0a127480bede2806206/zorg/buildbot/builders/OpenMPBuilder.py#L34
[3] https://github.com/llvm/llvm-zorg/blob/529cab62b53092cafe42c0a127480bede2806206/zorg/buildbot/builders/ClangBuilder.py#L267
[4] https://github.com/llvm/llvm-zorg/blob/529cab62b53092cafe42c0a127480bede2806206/zorg/buildbot/builders/ClangBuilder.py#L551

[Renato Golin via llvm-dev]

On Tue, 12 Oct 2021 at 21:20, Mircea Trofin <mtr...@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

[Mircea Trofin via llvm-dev]

Putting it together, it seems like we can start with unifying the 2 in llvm/libcxx, then figure llvm-test-suite along the lines discussed so far.

For the former, libcxx seems to have been updated most recently, I'll copy that one over to llvm/utils, seems benign, unless I'm missing something.

 

cheers,

--renato

[Mircea Trofin via llvm-dev]

Circling back from https://reviews.llvm.org/D112012 - would having google/benchmark under a new `llvm-project/third-party` work, instead of `llvm-project/llvm/utils`? (rationale there, TL;DR; libcxx (and other runtimes) wants to detach itself from anything `llvm-project/llvm`)

[Michael Kruse via llvm-dev]

Am Mo., 25. Okt. 2021 um 10:18 Uhr schrieb Mircea Trofin <mtr...@google.com>:
> Circling back from https://reviews.llvm.org/D112012 - would having google/benchmark under a new `llvm-project/third-party` work, instead of `llvm-project/llvm/utils`? (rationale there, TL;DR; libcxx (and other runtimes) wants to detach itself from anything `llvm-project/llvm`)

It would be only part of the solution as libcxx also depends on other
things in llvm-project/llvm (CMake modules, lit, ...). If the goal is
to make libcxx etc independent of the /llvm we should find out where
those other dependencies go. Until then, IMHO there is nothing wrong
to have libcxx depend on /llvm/utils/benchmark since it has /llvm is
needed to build libc++ anyway.

[Chris Lattner via llvm-dev]

> On Oct 29, 2021, at 7:00 AM, Michael Kruse <llv...@meinersbur.de> wrote:
>
> Am Mo., 25. Okt. 2021 um 10:18 Uhr schrieb Mircea Trofin <mtr...@google.com>:
>> Circling back from https://reviews.llvm.org/D112012 - would having google/benchmark under a new `llvm-project/third-party` work, instead of `llvm-project/llvm/utils`? (rationale there, TL;DR; libcxx (and other runtimes) wants to detach itself from anything `llvm-project/llvm`)
>
> It would be only part of the solution as libcxx also depends on other
> things in llvm-project/llvm (CMake modules, lit, ...). If the goal is
> to make libcxx etc independent of the /llvm we should find out where
> those other dependencies go. Until then, IMHO there is nothing wrong
> to have libcxx depend on /llvm/utils/benchmark since it has /llvm is
> needed to build libc++ anyway.

I think it would make sense to move third party dependencies out of llvm-project/llvm for other reasons too, including layering (they can’t depend on other things in llvm) and license clarity.

-Chris

[Mircea Trofin via llvm-dev]

On Sun, Oct 31, 2021 at 11:57 AM Chris Lattner <clat...@nondot.org> wrote:

> On Oct 29, 2021, at 7:00 AM, Michael Kruse <llv...@meinersbur.de> wrote:
>
> Am Mo., 25. Okt. 2021 um 10:18 Uhr schrieb Mircea Trofin <mtr...@google.com>:
>> Circling back from https://reviews.llvm.org/D112012 - would having google/benchmark under a new `llvm-project/third-party` work, instead of `llvm-project/llvm/utils`? (rationale there, TL;DR; libcxx (and other runtimes) wants to detach itself from anything `llvm-project/llvm`)
>
> It would be only part of the solution as libcxx also depends on other
> things in llvm-project/llvm (CMake modules, lit, ...). If the goal is
> to make libcxx etc independent of the /llvm we should find out where
> those other dependencies go. Until then, IMHO there is nothing wrong
> to have libcxx depend on /llvm/utils/benchmark since it has /llvm is
> needed to build libc++ anyway.

I think it would make sense to move third party dependencies out of llvm-project/llvm for other reasons too, including layering (they can’t depend on other things in llvm) and license clarity. 

sounds good, so I'll create a 'third-party' dir, unless there's a strong desire for a different name

 

 

-Chris

[Reid Kleckner via llvm-dev]

On Sun, Oct 31, 2021 at 11:57 AM Chris Lattner via llvm-dev <llvm...@lists.llvm.org> wrote:

I think it would make sense to move third party dependencies out of llvm-project/llvm for other reasons too, including layering (they can’t depend on other things in llvm) and license clarity. 

+1, gtest & gmock come to mind.

[Tom Stellard via llvm-dev]

On 11/1/21 10:39 AM, Reid Kleckner via llvm-dev wrote:

> On Sun, Oct 31, 2021 at 11:57 AM Chris Lattner via llvm-dev <llvm...@lists.llvm.org <mailto:llvm...@lists.llvm.org>> wrote:
>
> I think it would make sense to move third party dependencies out of llvm-project/llvm for other reasons too, including layering (they can’t depend on other things in llvm) and license clarity.
>
>
> +1, gtest & gmock come to mind.

I'm in favor of this, but gtest currently has a dependency on LLVMSupport.

-Tom

[via llvm-dev]

> On 11/1/21 10:39 AM, Reid Kleckner via llvm-dev wrote:

> > On Sun, Oct 31, 2021 at 11:57 AM Chris Lattner via llvm-dev <llvm-
> d...@lists.llvm.org <mailto:llvm...@lists.llvm.org>> wrote:
> >
> > I think it would make sense to move third party dependencies out of
> llvm-project/llvm for other reasons too, including layering (they can’t
> depend on other things in llvm) and license clarity.
> >
> >
> > +1, gtest & gmock come to mind.
>
> I'm in favor of this, but gtest currently has a dependency on LLVMSupport.

According to the README we have local patches to support a few targets that
upstream gtest doesn't, and also allow some LLVM types (StringRef etc) to be
used in the printing features.

--paulr

[Chris Lattner via llvm-dev]

> On Nov 1, 2021, at 12:15 PM, paul.r...@sony.com wrote:
>
>> On 11/1/21 10:39 AM, Reid Kleckner via llvm-dev wrote:
>>> On Sun, Oct 31, 2021 at 11:57 AM Chris Lattner via llvm-dev <llvm-
>> d...@lists.llvm.org <mailto:llvm...@lists.llvm.org>> wrote:
>>>
>>> I think it would make sense to move third party dependencies out of
>> llvm-project/llvm for other reasons too, including layering (they can’t
>> depend on other things in llvm) and license clarity.
>>>
>>>
>>> +1, gtest & gmock come to mind.
>>
>> I'm in favor of this, but gtest currently has a dependency on LLVMSupport.
>
> According to the README we have local patches to support a few targets that
> upstream gtest doesn't, and also allow some LLVM types (StringRef etc) to be
> used in the printing features.

Huh ok. Could that be handled by making them be a template or something?

-Chris

[via llvm-dev]

> > On Nov 1, 2021, at 12:15 PM, paul.r...@sony.com wrote:
> >
> >> On 11/1/21 10:39 AM, Reid Kleckner via llvm-dev wrote:
> >>> On Sun, Oct 31, 2021 at 11:57 AM Chris Lattner via llvm-dev <llvm-
> >> d...@lists.llvm.org <mailto:llvm...@lists.llvm.org>> wrote:
> >>>
> >>> I think it would make sense to move third party dependencies out of
> >> llvm-project/llvm for other reasons too, including layering (they can’t
> >> depend on other things in llvm) and license clarity.
> >>>
> >>>
> >>> +1, gtest & gmock come to mind.
> >>
> >> I'm in favor of this, but gtest currently has a dependency on
> LLVMSupport.
> >
> > According to the README we have local patches to support a few targets
> that
> > upstream gtest doesn't, and also allow some LLVM types (StringRef etc)
> to be
> > used in the printing features.
>
> Huh ok. Could that be handled by making them be a template or something?
>
> -Chris

Without taking a deep look, that doesn't seem to be how gtest expects to be
customized. The StringRef/SmallString stuff is in a header that's intended
to be modified by an installation, and consists of overloads that let the
gtest code DTRT when you do something like

EXPECT_EQ(a, b) << c;

and 'c' is a StringRef or SmallString.

I haven't tried to look into the OS support.
--paulr