1899 improve makerunnable allocation performance #1900

lifflander · 2022-08-05T18:02:38Z

nlslatt

Do you want to add the node_ to the debug prints in set_context.cc?

nlslatt · 2022-08-05T18:08:49Z

src/vt/pool/static_sized/memory_pool_equal.cc

@@ -146,3 +147,9 @@ template struct MemoryPoolEqual<memory_size_small>;
 template struct MemoryPoolEqual<memory_size_medium>;

 }} //end namespace vt::pool
+
+namespace vt { namespace pool {


Why close the namespace and reopen immediately?

Ok I fexed that.

nlslatt · 2022-08-05T18:19:58Z

src/vt/runnable/runnable.impl.h

+  for (int i = 0; i < ci_; i++) {
+    auto t = dynamic_cast<T*>(contexts_[i].get());


Given the limited number of context types, would it make sense to designate a specific slot to each context type, to minimize the number of dynamic_cast calls?

I refactored the code to have a separate member for each context type instead of an array. Is that something that you had in mind?

PhilMiller · 2022-08-05T18:24:16Z

Code looks reasonable enough. How much performance impact does this have?

github-actions · 2022-08-05T18:27:30Z

Pipelines results

PR tests (gcc-5, ubuntu, mpich)

Build for b138399

Compilation - successful

Testing - passed

Build log

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

Build for b138399

Compilation - successful

Testing - passed

Build log

PR tests (clang-5.0, ubuntu, mpich)

Build for b138399

Compilation - successful

Testing - passed

Build log

PR tests (clang-3.9, ubuntu, mpich)

Build for b138399

Compilation - successful

Testing - passed

Build log

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

Build for b138399

Compilation - successful

Testing - passed

Build log

PR tests (gcc-6, ubuntu, mpich)

Build for b138399

Compilation - successful

Testing - passed

Build log

PR tests (clang-9, ubuntu, mpich)

Build for b138399

Compilation - successful

Testing - passed

Build log

PR tests (clang-13, alpine, mpich)

Build for b138399

Compilation - successful

Testing - passed

Build log

PR tests (nvidia cuda 11.0, ubuntu, mpich)

Build for b138399

Compilation - successful

Testing - passed

Build log

PR tests (clang-12, ubuntu, mpich)

Build for b138399

Compilation - successful

Testing - passed

Build log

PR tests (nvidia cuda 10.1, ubuntu, mpich)

Build for b138399

Compilation - successful

Testing - passed

Build log

PR tests (clang-11, ubuntu, mpich)

Build for b138399

Compilation - successful

Testing - passed

Build log

PR tests (intel icpx, ubuntu, mpich)

Build for b138399

Compilation - successful

Testing - passed

Build log

PR tests (clang-13, ubuntu, mpich)

Build for b138399

Compilation - successful

Testing - passed

Build log

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

Build for 1942c6b

Compilation - successful

Testing - passed

Build log

PR tests (clang-14, ubuntu, mpich)

Build for b138399

Compilation - successful

Testing - passed

Build log

PR tests (gcc-11, ubuntu, mpich)

Build for b138399

Compilation - successful

Testing - passed

Build log

PR tests (gcc-12, ubuntu, mpich)

Build for b138399

Compilation - successful

Testing - passed

Build log

PR tests (clang-10, ubuntu, mpich)

Build for b138399

Compilation - successful

Testing - passed

Build log

PR tests (intel icpc, ubuntu, mpich)

Build for b138399

Compilation - successful

Testing - passed

Build log

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

Build for cbd764e

Compilation - successful

Testing - passed

Build log

PhilMiller · 2022-08-16T17:58:23Z

The gcc-7 CI build is repeatedly hanging the Azure agent at Linking CXX executable tests/atomic_basic

nlslatt · 2022-08-16T18:42:13Z

The gcc-7 CI build is repeatedly hanging the Azure agent at Linking CXX executable tests/atomic_basic

I saw many such hangs in this vicinity from my PR to change the number of ranks used for running the ping pong test, so the hang here may not be related to this PR. Any thoughts on how to figure out what's actually going wrong? Do you think we're exceeding memory in our parallel build (trace is enabled so it might be one of the tougher builds)?

codecov · 2022-08-24T11:46:56Z

Codecov Report

Merging #1900 (cbd764e) into develop (ff3ec68) will increase coverage by 0.02%.
The diff coverage is 73.52%.

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

@@             Coverage Diff             @@
##           develop    #1900      +/-   ##
===========================================
+ Coverage    84.39%   84.42%   +0.02%     
===========================================
  Files          761      763       +2     
  Lines        26866    26930      +64     
===========================================
+ Hits         22673    22735      +62     
- Misses        4193     4195       +2

Impacted Files	Coverage Δ
src/vt/context/runnable_context/collection.h	`100.00% <ø> (ø)`
src/vt/pool/static_sized/memory_pool_equal.cc	`100.00% <ø> (ø)`
src/vt/runnable/runnable.h	`77.77% <ø> (-2.23%)`	⬇️
src/vt/vrt/collection/manager.impl.h	`96.48% <ø> (ø)`
src/vt/context/runnable_context/collection.cc	`50.00% <50.00%> (ø)`
src/vt/runnable/runnable.cc	`71.42% <70.96%> (+0.69%)`	⬆️
src/vt/utils/ptr/unique_fixed.h	`76.92% <76.92%> (ø)`
src/vt/context/context.cc	`100.00% <100.00%> (ø)`
src/vt/context/runnable_context/collection.impl.h	`100.00% <100.00%> (+40.00%)`	⬆️
src/vt/context/runnable_context/set_context.h	`100.00% <100.00%> (ø)`
... and 50 more

PhilMiller · 2022-08-30T17:29:32Z

Superseded by later smaller PRs

lifflander linked an issue Aug 5, 2022 that may be closed by this pull request

Improve makeRunnable allocation performance #1899

Closed

3 tasks

lifflander requested review from nlslatt and PhilMiller August 5, 2022 18:02

nlslatt approved these changes Aug 5, 2022

View reviewed changes

lifflander requested a review from thearusable August 9, 2022 17:45

stmcgovern mentioned this pull request Aug 16, 2022

Use memory pool for RunnableNew #1914

Closed

nmm0 mentioned this pull request Aug 23, 2022

Meeting Agenda [do not close] #925

Open

thearusable force-pushed the 1899-improve-makerunnable-allocation-performance branch from cbd764e to 1942c6b Compare August 24, 2022 12:14

lifflander and others added 9 commits August 25, 2022 11:41

#1899: context: remove template from collection

70b4402

#1899: util: implement unique ptr with pool

7e07313

#1899: util: use new memory pool

ade8a7e

#1899: context: combine SetContext and FromNode

a28d6d0

#1899: Remove unnecessary namespace

5791a01

#1899: Split array into separate variables for each context type

b4fc0f1

#1899: Don't add trace context when tracing is disabled

b6803ac

#1899: Improve methods description in runnable.h

985a18c

#1899: Update formating with clang-format

b138399

thearusable force-pushed the 1899-improve-makerunnable-allocation-performance branch from 1942c6b to b138399 Compare August 25, 2022 09:52

PhilMiller closed this Aug 30, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

1899 improve makerunnable allocation performance #1900

1899 improve makerunnable allocation performance #1900

lifflander commented Aug 5, 2022

nlslatt left a comment

nlslatt Aug 5, 2022

thearusable Aug 23, 2022

nlslatt Aug 5, 2022

thearusable Aug 23, 2022

PhilMiller commented Aug 5, 2022

github-actions bot commented Aug 5, 2022 •

edited

Loading

PhilMiller commented Aug 16, 2022

nlslatt commented Aug 16, 2022

codecov bot commented Aug 24, 2022 •

edited

Loading

PhilMiller commented Aug 30, 2022

		for (int i = 0; i < ci_; i++) {
		auto t = dynamic_cast<T*>(contexts_[i].get());

1899 improve makerunnable allocation performance #1900

1899 improve makerunnable allocation performance #1900

Conversation

lifflander commented Aug 5, 2022

nlslatt left a comment

Choose a reason for hiding this comment

nlslatt Aug 5, 2022

Choose a reason for hiding this comment

thearusable Aug 23, 2022

Choose a reason for hiding this comment

nlslatt Aug 5, 2022

Choose a reason for hiding this comment

thearusable Aug 23, 2022

Choose a reason for hiding this comment

PhilMiller commented Aug 5, 2022

github-actions bot commented Aug 5, 2022 • edited Loading

Pipelines results

PhilMiller commented Aug 16, 2022

nlslatt commented Aug 16, 2022

codecov bot commented Aug 24, 2022 • edited Loading

Codecov Report

PhilMiller commented Aug 30, 2022

github-actions bot commented Aug 5, 2022 •

edited

Loading

codecov bot commented Aug 24, 2022 •

edited

Loading