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

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

My knowledge of this area is relatively limited, so I don't feel like I have a good enough understanding of the intricacies to progress this review further. Thanks for the explanations as to why this approach is most appropriate. The real cost sounds like it's due to process spawning overhead primarily (and any startup costs within the program itself). That does make me wonder whether there's a natural way of threading things within the test programs themselves, but I suspect not as I doubt the unit tests are thread-safe. Consequently, this approach seems sound, but hopefully others with knowledge in this area might be able to chime in?



================
Comment at: llvm/utils/lit/lit/main.py:143-144
+                idxs.append(idx)
+        for i in range(len(idxs)):
+            del tests[idxs[i]-i]
+
----------------
ychen wrote:
> jhenderson wrote:
> > Doesn't this invalidate the indexes after the first one? If we need to delete items 2 and 4 in the list, after deleting item 2, item 4 will be at index 3 now.
> The `-i` makes the index correct. To delete items 2 and 4 in the list, the `idxs` is `[2,4]`. The `del` statement ends up being `del tests[2-0]` and `del tests[4-1]`.
Ah, I misread that as `-1` not `-i`. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122251