1899 Improve performance of Runnable and surrounding machinery #1963

lifflander · 2022-09-14T17:03:25Z

Fixes #1899
Fixes #1593 (see commit 4992e78)

github-actions · 2022-09-14T18:02:31Z

Pipelines results

PR tests (gcc-12, ubuntu, mpich)

Build for 8568fb7

Compilation - successful

Testing - passed

Build log

PR tests (gcc-5, ubuntu, mpich)

Build for 8568fb7

Compilation - successful

Testing - passed

Build log

PR tests (clang-3.9, ubuntu, mpich)

Build for 8568fb7

Compilation - successful

Testing - passed

Build log

PR tests (gcc-10, ubuntu, openmpi, no LB)

Build for 8568fb7

Compilation - successful

Testing - passed

Build log

PR tests (clang-5.0, ubuntu, mpich)

Build for 8568fb7

Compilation - successful

Testing - passed

Build log

PR tests (gcc-7, ubuntu, mpich, trace runtime, LB)

Build for 8568fb7

Compilation - successful

Testing - passed

Build log

PR tests (gcc-8, ubuntu, mpich, address sanitizer)

Build for 8568fb7

Compilation - successful

Testing - passed

Build log

PR tests (gcc-9, ubuntu, mpich, zoltan)

Build for 8568fb7

Compilation - successful

Testing - passed

Build log

PR tests (nvidia cuda 10.1, ubuntu, mpich)

Build for 8568fb7

Compilation - successful

Testing - passed

Build log

PR tests (nvidia cuda 11.0, ubuntu, mpich)

Build for 8568fb7

Compilation - successful

Testing - passed

Build log

PR tests (gcc-6, ubuntu, mpich)

Build for 8568fb7

Compilation - successful

Testing - passed

Build log

PR tests (clang-10, ubuntu, mpich)

Build for 8568fb7

Compilation - successful

Testing - passed

Build log

PR tests (clang-9, ubuntu, mpich)

Build for 8568fb7

Compilation - successful

Testing - passed

Build log

PR tests (clang-12, ubuntu, mpich)

Build for 8568fb7

Compilation - successful

Testing - passed

Build log

PR tests (intel icpx, ubuntu, mpich)

Build for 8568fb7

Compilation - successful

Testing - passed

Build log

PR tests (clang-13, ubuntu, mpich)

Build for 8568fb7

Compilation - successful

Testing - passed

Build log

PR tests (clang-14, ubuntu, mpich)

Build for 8568fb7

Compilation - successful

Testing - passed

Build log

PR tests (gcc-11, ubuntu, mpich, json schema test)

Build for 8568fb7

Compilation - successful

Testing - passed

Build log

PR tests (intel icpc, ubuntu, mpich)

Build for 8568fb7

Compilation - successful

Testing - passed

Build log

PR tests (clang-11, ubuntu, mpich)

Build for 8568fb7

Compilation - successful

Testing - passed

Build log

PR tests (clang-13, alpine, mpich)

Build for 8568fb7

Compilation - successful

Testing - passed

Build log

codecov · 2022-09-20T20:01:18Z

Codecov Report

Merging #1963 (c7df3b9) into develop (3c555c0) will decrease coverage by 0.00%.
The diff coverage is n/a.

❗ Current head c7df3b9 differs from pull request most recent head 8568fb7. Consider uploading reports for the commit 8568fb7 to get more accurate results

@@             Coverage Diff             @@
##           develop    #1963      +/-   ##
===========================================
- Coverage    84.37%   84.37%   -0.01%     
===========================================
  Files          727      727              
  Lines        25491    25491              
===========================================
- Hits         21508    21507       -1     
- Misses        3983     3984       +1

Impacted Files	Coverage Δ
src/vt/messaging/envelope/payload_envelope.h	`73.52% <ø> (ø)`
src/vt/termination/termination.cc	`70.14% <ø> (ø)`
src/vt/pool/pool.cc	`87.50% <0.00%> (-1.05%)`	⬇️

lifflander · 2022-09-20T21:04:20Z

Looks like we have a double free when fcontext is enabled.

This is a long-standing bug that started showing up with the changes in #1899, making the runnable more efficient. TestGroup.test_group_range_construct_2 was consistently breaking on the gcc-8 build with address sanitizer. With extensive debugging, I tracked this bug down to the lack of propagation of an epoch on the remote group construction message when a rooted group is constructed where the constructing node is not included. Additionally, the group must be specified by a list (not a range). When these conditions occur, the group manager sends the information about the group to a remote node using a packed put, which uses the `PayloadMessage`: namely, ` struct GroupListMsg : GroupInfoMsg<GroupMsg<::vt::PayloadMessage>>`. The `PayloadMessage`, using a default template parameter, uses a basic envelope which is not large enough to store the epoch. Thus, it gets dropped. Therefore, the test sometimes breaks because the group construction and following broadcast escape the `runInEpochCollective` and the test condition will fail sometimes as it races with the delivery of the group message being broadcast to the group spanning tree.

stmcgovern

Lots going on here, but looks good to me!

…-allocation-performance-new-variant 1899 Improve performance of Runnable and surrounding machinery

lifflander requested review from PhilMiller, stmcgovern, cz4rs, nmm0, JacobDomagala, jstrzebonski and nlslatt September 14, 2022 17:03

lifflander changed the title ~~1899 improve makerunnable allocation performance new variant~~ 1899 Improve performance of Runnable and surroundering machinery Sep 15, 2022

lifflander force-pushed the 1899-improve-makerunnable-allocation-performance-new-variant branch 3 times, most recently from d411151 to 3e4334b Compare September 20, 2022 15:27

lifflander marked this pull request as ready for review September 20, 2022 15:27

stmcgovern changed the title ~~1899 Improve performance of Runnable and surroundering machinery~~ 1899 Improve performance of Runnable and surrounding machinery Sep 20, 2022

stmcgovern mentioned this pull request Sep 21, 2022

Use memory pool for RunnableNew #1914

Closed

lifflander force-pushed the 1899-improve-makerunnable-allocation-performance-new-variant branch from 5416018 to bc3b2b4 Compare September 22, 2022 17:42

lifflander added 12 commits September 27, 2022 10:06

#1899: context: combine SetContext and FromNode

852db4b

#1899: context: remove template from collection

aadcc8c

#1899: runnable: store contexts directly instead of using pointers

888e88e

#1899: runnable: fix TD construct/destruct

486e858

#1899: runnable: remove Base class for contexts

902a733

#1899: runnable: allocate through pool

0192e55

#1899: runnable: use raw pointer for RunnableNew

2111af5

#1899: runnable: fix linking problem

3d03ee5

#1899: runnable: switch to bare pointer

f7dbca2

#1899: registry: completely remove old manual registry

9f99fe3

#1899: runnable: remove tag as a runnable parameter

a2812e0

#1899: active: fix debug print

ca3c4b5

lifflander added 14 commits September 27, 2022 10:06

#1899: runnable: make message const& in builder

bc52656

#1899: runnable: disable more fcontext code when compile-time disabled

1866bba

#1899: runnable: do not reference message to enqueue

53e6512

#1899: td: disable suspended epochs when fcontext is disabled

4473309

#1899: td: take message by const&

77b053b

#1899: scheduler: don't transition to idle right away

892c29a

#1899: scheduler: fix bad rebase with endif

087d68e

#1899: runnable: add missing getter for trace context

3b3d77d

#1899: runnable: fix missing endif from merge

7fbf2d2

#1899: runnable: fix double free

b54d08e

#1899: runnable: stop accessing msg_ if const& is invalid

cdd8cf0

#1899: runnable: add missing end for continuation causing leak

69fb406

#1899: TD: make testEpochTerminated print verbose (mis-marked)

ba19bad

lifflander force-pushed the 1899-improve-makerunnable-allocation-performance-new-variant branch from c7df3b9 to ba19bad Compare September 27, 2022 17:06

nmm0 mentioned this pull request Sep 27, 2022

Meeting Agenda [do not close] #925

Open

#1899: runnable: try moving trace context section up to fix Mac bug

8568fb7

stmcgovern approved these changes Sep 27, 2022

View reviewed changes

lifflander merged commit c724cf0 into develop Sep 27, 2022

cz4rs pushed a commit that referenced this pull request Sep 28, 2022

Merge pull request #1963 from DARMA-tasking/1899-improve-makerunnable…

81c1c08

…-allocation-performance-new-variant 1899 Improve performance of Runnable and surrounding machinery

lifflander mentioned this pull request Oct 4, 2022

Remove old swap handler code #1911

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

1899 Improve performance of Runnable and surrounding machinery #1963

1899 Improve performance of Runnable and surrounding machinery #1963

lifflander commented Sep 14, 2022 •

edited

Loading

github-actions bot commented Sep 14, 2022 •

edited

Loading

codecov bot commented Sep 20, 2022 •

edited

Loading

lifflander commented Sep 20, 2022

stmcgovern left a comment

1899 Improve performance of Runnable and surrounding machinery #1963

1899 Improve performance of Runnable and surrounding machinery #1963

Conversation

lifflander commented Sep 14, 2022 • edited Loading

github-actions bot commented Sep 14, 2022 • edited Loading

Pipelines results

codecov bot commented Sep 20, 2022 • edited Loading

Codecov Report

lifflander commented Sep 20, 2022

stmcgovern left a comment

Choose a reason for hiding this comment

lifflander commented Sep 14, 2022 •

edited

Loading

github-actions bot commented Sep 14, 2022 •

edited

Loading

codecov bot commented Sep 20, 2022 •

edited

Loading