sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.31k stars 449 forks source link

Don't sort files by (previous) doctest running time (which is wall time!) by default #14604

Open 83660e46-0051-498b-a8c1-f7a7bd232b5a opened 11 years ago

83660e46-0051-498b-a8c1-f7a7bd232b5a commented 11 years ago

Doing so is just pretty stupid, because if doctesting file A, B, C took very long (or even timed out) because A, B, C eat up quite a lot of resources (e.g. memory, or do heavy I/O), running them simultaneously next time will just worsen the situation. (This of course applies to parallel doctesting only, i.e., when SAGE_NUM_THREADS >1.)

Also, files that take only fractions of a second to test will all be doctested at the same time (i.e., last), which in turn causes undesirable I/O peaks.

Component: doctest framework

Keywords: ptestlong timeout time-out timed

Issue created by migration from https://trac.sagemath.org/ticket/14604

83660e46-0051-498b-a8c1-f7a7bd232b5a commented 11 years ago
comment:1

If at all, "long" and "short" tests should get mixed, i.e., run at the same time.

jdemeyer commented 11 years ago

Replying to @nexttime:

files that take only fractions of a second to test will all be doctested at the same time (i.e., last), which in turn causes undesirable I/O peaks.

Really? Why would that cause I/O peaks? Sage is imported before forking.

83660e46-0051-498b-a8c1-f7a7bd232b5a commented 11 years ago
comment:3

Replying to @jdemeyer:

Replying to @nexttime:

files that take only fractions of a second to test will all be doctested at the same time (i.e., last), which in turn causes undesirable I/O peaks.

Really? Why would that cause I/O peaks? Sage is imported before forking.

Well, meanwhile (with the new doctesting framework), at least the terminal (and the logfile) gets flooded ... :-)

Besides that, as far as I know, not everything a doctest may use gets imported in advance. (And doctests still have to get extracted from the sources, don't they?)

vbraun commented 11 years ago
comment:4

The standard lore for "bag of tasks" parallelism is to do the long ones first and then backfill with the shorter tasks. Its true that this might cause memory issues if long-running = lots of memory consumption task. But I'm not sure thats true in the Sage library, some doctests are just slow because they sleep (e.g. to test alarm).

83660e46-0051-498b-a8c1-f7a7bd232b5a commented 11 years ago
comment:5

Replying to @vbraun:

The standard lore for "bag of tasks" parallelism is to do the long ones first and then backfill with the shorter tasks.

Nobody would schedule tasks blindly by just wall time they once took (for whatever, ignored reason), at least not if they use the same set of other resources, hence can't run independently of each other.

Its true that this might cause memory issues if long-running = lots of memory consumption task. But I'm not sure thats true in the Sage library

It is true, although not in the sense of a one-to-one correspondence (and gets even worse because a few people seem pretty ignorant concerning memory consumption or runtime -- or both at the same time -- of [long] doctests); just try...

some doctests are just slow because they sleep (e.g. to test alarm).

That's not the problem. (They shouldn't sleep for minutes; some appear to do so though...)

Note also that a couple of doctests use more than one thread (or, more precisely, process); running these simultaneously is especially bad (also w.r.t. memory consumption or more generally, access), at least if they already tend to take longer because of that.

The only benefit of sorting doctests by wall time (on rather resource-unlimited machines) is that some rare, broken tests that idle most of the time (like sandpile.py on some systems) get started early such that the overall test time (again wall time) might decrease; on such systems, one should explicitly ask for doing so. And this of course only works if the sysload is almost constant during testing.

Currently, there's not even an option to disable this. (One has to search and delete the rather hidden timings of previous tests.)

83660e46-0051-498b-a8c1-f7a7bd232b5a commented 11 years ago
comment:6

Replying to @nexttime:

The only benefit of sorting doctests by wall time (on rather resource-unlimited machines) is that some rare, broken tests that idle most of the time (like sandpile.py on some systems) get started early such that the overall test time (again wall time) might decrease;

If you have multiple of such tests and start all these first, the effect is of course again counter-productive, as then the machine will first idle most of the time, afterwards running all "busy" ones together.

roed314 commented 11 years ago
comment:7

Running doctests sorted by walltime is most useful when you're just doctesting one folder: imagine you have two processes, one task that takes 100 seconds and 10 tasks that take 10 seconds. You certainly want to start the 100 second task first.

In the case of the whole Sage library, I would like to see some data from different machines on whether sorting the tasks is good, bad or neutral.

Adding an option to disable the sorting is certainly reasonable if there are cases where it's counterproductive. What the default should be is another matter.

vbraun commented 11 years ago
comment:8

Agree, there may be better ways to sort. But not sorting is crap.

83660e46-0051-498b-a8c1-f7a7bd232b5a commented 11 years ago
comment:9

Replying to @vbraun:

Agree, there may be better ways to sort. But not sorting is crap.

Well, you at least get a "natural" mixture when not sorting at all (and hence better results on averageTM).

(IIRC, not sure whether that's still true for the new doctesting framework, but even when doing e.g. sage -t file1 file2 ... the files got reordered, not necessarily by previous doctest time.)

83660e46-0051-498b-a8c1-f7a7bd232b5a commented 11 years ago
comment:10

P.S.: Cf. the discussion at #2695.