make CI tests faster (2.4X faster on local OSX), and make gcleak2 leak detection more accurate

[timotheecour]

before PR

there's a single test that takes a whopping 335 seconds (on my local OSX; on other machines may be different), representing > 57% of CI tests time (585 s) (ie, all of testament/testament all; this excludes other things in ./koch runCI).

on local OSX:

XDG_CONFIG_HOME= nim c -r testament/testament --nim:./bin/nim all
PASS: tests/gc/gcleak2 C --gc:boehm                                (165.00503802 secs)
PASS: tests/gc/gcleak2 C -d:release --gc:boehm                     (170.92052507 secs)
...

after PR

• these:
  ./bin/nim c -r -d:release --gc:boehm tests/gc/gcleak2.nim
  ./bin/nim c -r --gc:boehm tests/gc/gcleak2.nim
  now both run in 1 sec, by simply reducing number of iterations in this case; the test is in fact more useful than before despite smaller numIter, as we check that the occupied memory doesn't vary after few iterations; so running more iterations would make no difference.

• other deterministic gc tested (eg: --gc:orc, --gc:arc) also are tested like --gc:boehm (ie fixed point at a few iterations)

• for the other gc tested (ie ./bin/nim c -r tests/gc/gcleak2.nim + other gc) , we also modify the test to make it more useful, by checking that occupied memory follows a set of cycles, each with identical min/max values after a few cycles. The test before this PR was more rough, and would not have detected a small leak accumulating since it just had a coarse upperbound on memory use.

and overall, testament/testament all now runs in 242s, a 2.4X speedup (on my local OSX)

Note

on azure CI machines, the overall speedup is less significant compared to the speedup observed on my local OSX machine (OSX catalina 10.15, 16GB RAM 2.3 GHz 8-Core Intel Core i9); for some reason ./bin/nim c -r --gc:boehm tests/gc/gcleak2.nim was 165s (before this PR) on my local machine but only 65s on azure OSX machines (note: azure uses 10.14 not 10.15, could be a factor) eg:
https://dev.azure.com/nim-lang/255dfe86-e590-40bb-a8a2-3c0295ebdeb1/_apis/build/builds/2835/logs/60

2020-02-24T22:22:05.2156800Z �[32mPASS: �[36mtests/gc/gcleak2 C --gc:boehm                               �[34m (65.62197399 secs)�[0m
2020-02-24T22:22:05.2157710Z �[32mPASS: �[36mtests/gc/gcleak2 C -d:release --gc:boehm                    �[34m (54.69007993 secs)�[0m

still, this PR saves 120s on each such azure machine, and it saves 335s (a 2.4X overall speedup) on a local OSX machine, and makes the tests more accurate.

[narimiran]

(2.4X faster on local OSX)

on azure CI machines, the overall speedup is less significant

was (...) only 65s on azure OSX machines

still, this PR saves 120s on each such azure machine

Ok, I'll be @krux02 this time: can you give us a straight-forward non-clickbait non-convoluted realistic numbers for our CIs? (We don't care about your local machine numbers)

[timotheecour]

in absolute terms, on azure CI machines, it saves 120s per machine; it's the slowest test

We don't care about your local machine numbers

unless other people running latest OSX (10.15) have same issue of nim c -r -d:release --gc:boehm tests/gc/gcleak2.nim being so slow; I'd like someone with 10.15 to report the runtime for that command so I know it's not just my machine; I often run CI locally and the time saving is very large here. If ithe large local OSX speedup is just for me and not reproducible, I will change the title accordingly.

[Araq]

But now the test is much harder to understand. Why not simply change the number 1_000_000 to 10_000 instead and call it day? It used to be green with one million for months if not for years, no need to spend so much effort on it.

[krux02]

First of all, thank you for tackling this problem. This is really an important problem to solve. Slow CI is something that affects everybody and faster CI feedback is oil in the Nim development machinery. So for me a PR tacking this problem is high priority.

But what I don't like is, might be subjective but I like minimalism when it comes to the introduction of if and when branches, and you've introduced three when isDeterministic alone. I also really don't like variable declarations behind a when statement, as they create a context where variables might not be declared and you have to use more when statements later on (as you did in this pr). I think you can do better about it. And here I think your can use compile time polymorphism effeciently.

An idea that would be this:

var tester: (when isDeterministic: DeterministicTester else: HeuristicTester)

type 
  HeuristicTester = object
    numCycle
    memPrevious: int
    memMin: int
    numCollections: int
  [...]

proc measure(tester: HeuristicTester; mem: int) = [...]
proc measure(tester: DeterministicTester; mem: int) = [...]

[...]
   tester.measure(mem) # instead of when isDeterministinc 

[Clyybber]

I don't think this extra complexity is worth it, just to save 2 minutes.

[alehander92]

not sure about the complexity, but 2 minutes seems as a good win?

[narimiran]

but 2 minutes seems as a good win?

...which might be achieved by changing one number in one line, as shown above, without the extra complexity.

[timotheecour]

PTAL

• reverted changes (maybe don't squash so at least we have it in git history)
• then re-enabled simpler change that gives same speedup (335s on local OSX, 120s on azure machine)
• then did similar modification for ./tests/gc/gcleak.nim (saves another 33s on local OSX)

note

yes the original changes were more complex, but they were also more precise, checking for loop/cycle invariants as opposed to current situation where we're just checking for a very coarse 1-size-fits-all upperbound on used memory (across OS/CPU/optimization level; and especially across GC algorithms despite them being widely different)
Whereas the test I had written originally checked that:

• for some GC algos, memory usage was constant per iteration i after i >= 3
• for the other algos, memory usage followed a set of cycles with constant min/max memory usage after seeing 5 cycles

Which should find even the smallest leak should that occur in that code.

It used to be green with one million for months if not for years, no need to spend so much effort on it.

the problem isnt' that it's green; the problem is that it can be too loose to detect some leaks. Leaks do happen eg: these are 3 different leak issues

• fix #10488 GC memory leak regression #10498 etc (where I analyzed cycles eg see https://user-images.githubusercontent.com/2194784/51887771-bbb07800-2349-11e9-8236-c0a9af338689.png which shows cycles with slowly (depending on scale) increasing min/max values for each cycle)
• GC bug: seems very slow where it shouldn't; maybe it leaks? #10040
• --gc:arc doesn't detect cycle, leaks memory; {.cursor.} ineffective with seq #13080
  whether or not the modified files would've detected those is unsure, but it would've at least helped diagnose them since it identifies the cycles/min/max