[PATCH] D97046: [lit] Generalize `early_tests`

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini added a comment.

In D97046#2599840 <https://reviews.llvm.org/D97046#2599840>, @davezarzycki wrote:

> In D97046#2598124 <https://reviews.llvm.org/D97046#2598124>, @mehdi_amini wrote:
>
>> In D97046#2587436 <https://reviews.llvm.org/D97046#2587436>, @lebedev.ri wrote:
>>
>>> I feel like this should be handled by lit transparently, like recording the time it took to run each test last time, then somehow deciding on the new test run order based on that,
>>
>> +1.
>>
>> This seems like the right thing to do to me. The "early" or phase looks very ad-hoc to me. 
>> It also isn't clear to me how it'd would play with a "recording time and replay" solution that would be the desired default for me.
>
> I don't know if you saw my response to @lebedev.ri but I'll try to paraphrase: I agree that recording and then sorting the order would be ideal for people doing incremental builds/testing but it won't help clean builds (bots and paranoid people) unless that sorted list gets committed back to the repository.

I saw that, but I don't see it as a problem :)

1. clean build is a one-time thing, where most of the time will be spent building the compiler rather than running the tests
2. bots can keep the timing from last run on the system if needed.
3. I don't know what "paranoid" there is here? This is about having a text file with timing on the system, can you clarify? Ultimately if someone is paranoid about this they can pay the extra cost of not using the feature.

> I'm not looking to propose that anybody do that or that it be the only way to get good testing performance. Some people just want a low maintenance config that they update maybe once or twice a year at most. I.e. the top five or so slowest tests (and maybe in the case of Swift, putting the compiler crasher directories towards the end).
> If somebody wants to teach lit how to record a sorted list of tests times and then read that list back in again, that'd be great, but I think that's a great followup patch. Thanks!

Sure, I'm mostly concern that 1) it isn't clear how this plays with reading back the test time (would they override the "start-phases"?) and how the "start-phases" can encourage this to spread much further than the occasional "top five" slowest tests.



================
Comment at: mlir/test/Unit/lit.cfg.py:17
+# start_phase; Request to start this suite early.
+config.start_phase = -1
 
----------------
What is the motivation for MLIR Unittests to start early?
I'd expect this to be at least documented. The comment here is just repeating *what* the code is doing instead of *why* we're doing this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97046