[PATCH] D122251: [lit] Use sharding for GoogleTest format

[Yuanfang Chen via Phabricator via llvm-commits]

ychen added a comment.

In D122251#3401843 <https://reviews.llvm.org/D122251#3401843>, @jhenderson wrote:

> In D122251#3401008 <https://reviews.llvm.org/D122251#3401008>, @ychen wrote:
>
>> Do we support using upstream googletest instead of the in-tree copy for the unit test?
>
> LLVM unit tests use the in-tree gtest. If you need more recent functionality, you should look at updating the im-tree copy.
>
> What's with all the deleted files?

Hmm, I was asking about the upstream googletest because of the deleted tests. They were introduced by D18606 <https://reviews.llvm.org/D18606> to handle upstream googletest `--gtest_list_tests` output. This is not needed for sharding since `--gtest_list_tests` is only used for estimating the number of sub-testcases which in turn is used to compute the number of shards. The correctness is always preserved.



================
Comment at: llvm/utils/lit/lit/formats/googletest.py:39
                             litConfig, localConfig):
+        init_shard_size = 512 # number of tests in a shard
+        core_count = lit.util.usable_core_count()
----------------
jhenderson wrote:
> Why 512?
> Why 512?

Admittedly, I chose this with LLVM lit testing in mind. I'm not sure this is a good number for the general use of LIT. The criteria are that the number should be as big as possible to keep the number of shards small (so that the overhead is small) while the number of shards should be big enough to keep all cores busy and not hurts scheduling.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122251/new/

https://reviews.llvm.org/D122251