Never run benchmarks concurrently, always output to console #3663#3666
Never run benchmarks concurrently, always output to console #3663#3666
Conversation
|
I quite like that now all benchmark output will be visible. Might consider doing this for tests too if concurrent-tests is set to |
mgsloan
force-pushed
the
fix-parallel-final-tasks-3663
branch
from
e3d0f9c to
3db9fc6
Compare
ChangeLog.md
Outdated
| * Run the Cabal file checking in the `sdist` command more reliably by | ||
| allowing the Cabal library to flatten the | ||
| `GenericPackageDescription` itself. | ||
| * Benchmarks used to be run concurrently with other benchmarks |
There was a problem hiding this comment.
I'm assuming the reason for this is to not confuse the benchmark results with other CPU activities, right? May be worth pointing that out.
| sortActions :: [Action] -> [Action] | ||
| sortActions = sortBy (compareConcurrency `on` actionConcurrency) | ||
| where | ||
| compareConcurrency ConcurrencyAllowed ConcurrencyDisallowed = LT |
There was a problem hiding this comment.
What about deriving Ord on the Concurrency type?
There was a problem hiding this comment.
I considered that, but it would be easy for someone to add a constructor and not realize its impact on the usage of Ord. I've added a note about it.
mgsloan
force-pushed
the
fix-parallel-final-tasks-3663
branch
from
3db9fc6 to
f0f2a8b
Compare
Also generally cleans up code related to parallel execution of tasks. Instead of locking happening among "final tasks" (tests and benchmark running), it's now possible to mark some tasks as work that shouldn't be done in parallel with anything else. This is what makes sense for benchmark running - they shouldn't be run concurrently with either building or running tests. Previously benchmarks and tests shared the same final task. The mechanism to execute one task exclusively is part of Control.Concurrent.Execute. If they were kept in the same task, then if any benchmarks were enabled, then tests would be run without any concurrency. In order to have as much concurrency as possible, they are now split into two different "final" tasks.
|
@snoyberg I've addressed your feedback. I'm particularly interested to know if you like the change to default to outputting benchmark results? |
mgsloan
force-pushed
the
fix-parallel-final-tasks-3663
branch
from
f0f2a8b to
4346cfd
Compare
snoyberg
left a comment
snoyberg
left a comment
There was a problem hiding this comment.
Seems like a good change to me, no objection.
3 participants
Also generally cleans up code related to parallel execution of tasks. Instead of
locking happening among "final tasks" (tests and benchmark running), it's now
possible to mark some tasks as work that shouldn't be done in parallel with
anything else. This is what makes sense for benchmark running - they shouldn't
be run concurrently with either building or running tests.
Previously benchmarks and tests shared the same final task. The mechanism to
execute one task exclusively is part of Control.Concurrent.Execute. If they were
kept in the same task, then if any benchmarks were enabled, then tests would be
run without any concurrency. In order to have as much concurrency as possible,
they are now split into two different "final" tasks.
Note: Documentation fixes for https://docs.haskellstack.org/en/stable/ should target the "stable" branch, not master.
Please include the following checklist in your PR:
Tested via the repro in #3663