Proposal: introduce dependency on abseil when building benchmarks

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.

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

Thanks!

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

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)

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,

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.

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.

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

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.

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

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.

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.

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?

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.

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)

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.

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!

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

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.

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