1051: Add local invoke to proxies that trace and record LB #1132

JacobDomagala · 2020-10-29T20:56:32Z

virtual collection proxy
obj group proxy
unittests

codecov · 2020-10-29T21:38:25Z

Codecov Report

Merging #1132 (e1d4ad4) into develop (8b6f242) will increase coverage by 1.34%.
The diff coverage is 98.57%.

@@             Coverage Diff             @@
##           develop    #1132      +/-   ##
===========================================
+ Coverage    79.61%   80.96%   +1.34%     
===========================================
  Files          724      729       +5     
  Lines        27809    28022     +213     
===========================================
+ Hits         22141    22687     +546     
+ Misses        5668     5335     -333

Impacted Files	Coverage Δ
src/vt/objgroup/manager.h	`100.00% <ø> (ø)`
src/vt/objgroup/proxy/proxy_objgroup_elm.h	`100.00% <ø> (ø)`
src/vt/vrt/collection/balance/lb_comm.h	`76.08% <ø> (ø)`
src/vt/vrt/collection/balance/node_stats.cc	`71.18% <ø> (ø)`
src/vt/vrt/collection/manager.h	`100.00% <ø> (ø)`
.../vt/vrt/collection/proxy_traits/proxy_elm_traits.h	`100.00% <ø> (ø)`
src/vt/runnable/invoke.h	`96.49% <96.49%> (ø)`
src/vt/vrt/collection/manager.impl.h	`94.82% <97.91%> (+0.14%)`	⬆️
src/vt/objgroup/manager.impl.h	`93.57% <100.00%> (+0.71%)`	⬆️
src/vt/objgroup/manager.static.h	`100.00% <100.00%> (ø)`
... and 23 more

JacobDomagala · 2020-11-23T15:00:22Z

src/vt/trace/trace_constants.h

@@ -83,7 +83,8 @@ enum class eTraceConstants : int32_t {
  UserStat                  = 32,
  BeginUserEventPair        = 98,
  EndUserEventPair          = 99,
-  UserEventPair             = 100
+  UserEventPair             = 100,
+  LocalInvoke               = 101


Not sure whether we want (or can as this probably won't work with Projections tool) to add new trace type for invoke or should we use existing one? At the end the output will be the same as for Creation event type.

Let's make this just a regular Creation event.

Projections doesn't do anything special for these.

lifflander · 2020-11-24T20:43:28Z

Per @PhilMiller suggestion, we should create a makeTraceCreationSend(..) for both starting the local invocation and on the receiving side after the invocation returns. So it will looks like a message was sent to start the local invoke and then a message was sent back to the same node to continue the work after the local invocation.

lifflander

Overall, this looks great and it's nearly ready to merge I think. Just a little more doxygen to be added to the proxy interface for virtual collections. Also, we should just use the standard trace event.

lifflander · 2020-11-24T20:45:19Z

src/vt/trace/trace_constants.h

@@ -83,7 +83,8 @@ enum class eTraceConstants : int32_t {
  UserStat                  = 32,
  BeginUserEventPair        = 98,
  EndUserEventPair          = 99,
-  UserEventPair             = 100
+  UserEventPair             = 100,
+  LocalInvoke               = 101


Let's make this just a regular Creation event.

lifflander · 2020-11-24T20:46:46Z

src/vt/trace/trace_constants.h

@@ -83,7 +83,8 @@ enum class eTraceConstants : int32_t {
  UserStat                  = 32,
  BeginUserEventPair        = 98,
  EndUserEventPair          = 99,
-  UserEventPair             = 100
+  UserEventPair             = 100,
+  LocalInvoke               = 101


Projections doesn't do anything special for these.

lifflander · 2020-11-24T20:47:21Z

src/vt/vrt/collection/invoke/invokable.h

+
+namespace vt { namespace vrt { namespace collection {
+
+template <typename ColT, typename IndexT, typename BaseProxyT>


Can you add doxygen for these new methods here?

JacobDomagala · 2020-11-30T20:30:45Z

@PhilMiller @lifflander Currently invoke produces following traces:

Creation event (let's say with trace_id == 2)

Begin Processing (trace_id == 2)
execute user's function

End Processing (trace_id == 2)

So you're proposing we should add another Creation event like following?

Creation event (let's say with trace_id == 2)

Begin Processing (trace_id == 2)
execute user's function

End Processing (trace_id == 2)

Creation event (trace_id == 3)

Or do you have something else in mind?

JacobDomagala · 2020-12-07T18:37:44Z

PR updated

lifflander · 2020-12-16T02:27:05Z

PR updated

Looks good I think this is ready to rebase and merge

…y which instantly invokes the handler without going through scheduler

…iven handler inline without going through scheduler

…data for given Callable object. Works for C style functions, functors and member functions

… proxy.

…invoke's Callable

…possible

… invoke function

…tion) to separate file

…oke function from trace module

JacobDomagala force-pushed the 1051-add-local-invoke-for-proxies branch 4 times, most recently from a274bae to 73fd8ab Compare November 4, 2020 19:54

JacobDomagala force-pushed the 1051-add-local-invoke-for-proxies branch from 73fd8ab to f005cf3 Compare November 16, 2020 22:21

lifflander mentioned this pull request Nov 17, 2020

Meeting Agenda [do not close] #925

Open

JacobDomagala force-pushed the 1051-add-local-invoke-for-proxies branch 3 times, most recently from dcf750b to 8f01fe4 Compare November 22, 2020 19:01

JacobDomagala commented Nov 23, 2020

View reviewed changes

JacobDomagala marked this pull request as ready for review November 24, 2020 18:16

JacobDomagala requested review from lifflander, nlslatt, cz4rs, jstrzebonski, nmm0 and PhilMiller November 24, 2020 18:39

lifflander approved these changes Nov 24, 2020

View reviewed changes

JacobDomagala force-pushed the 1051-add-local-invoke-for-proxies branch from 0523d66 to 30a7749 Compare November 28, 2020 19:03

JacobDomagala force-pushed the 1051-add-local-invoke-for-proxies branch 2 times, most recently from 48b62ed to 139b844 Compare December 3, 2020 19:20

JacobDomagala force-pushed the 1051-add-local-invoke-for-proxies branch 2 times, most recently from ed923bd to 3cfcf22 Compare December 14, 2020 20:41

JacobDomagala added 2 commits December 16, 2020 08:47

#1051: vrt-collection: add invoke function to virtual collection prox…

3090282

…y which instantly invokes the handler without going through scheduler

#1051: objgroup: add invoke function for objgrp proxy which invokes g…

97999ad

…iven handler inline without going through scheduler

JacobDomagala added 16 commits December 16, 2020 08:47

#1051: vrt-collection: Add unittest for Vrt-Collection invoke

b836975

#1051: objgroup: Add unittest for Objgroup proxy invoke

baf591a

#1051: runnable: Add invoke function which invokes and creates trace …

d790d80

…data for given Callable object. Works for C style functions, functors and member functions

#1051: objgroup: Add non-msg invoke function for objgrp proxy.

7e4fa4a

#1051: vrt-collection: Add non-msg invoke function for vrt-collection…

798dea1

… proxy.

#1051: runnable: Fix issue with incorrectly generated Trace name for …

d6b793b

…invoke's Callable

#1051: runnable: Use explicit return time for invoke.h function when …

1b43d37

…possible

#1051: vrt-collection: Cleanup invoke related code in vrt-collection

e937a15

#1051: runnable: Extract functions for trace names generation used in…

6d51393

… invoke function

#1051: runnable: Add more unittests for invoke function

20c78cc

#1051: runnable: Move return type SFINAE checks (used for invoke func…

db4ce8b

…tion) to separate file

#1051: objgroup: Remove warnings

9841046

#1051: runnable: Use LocalInvoke enum value in outputTraces function

2507946

#1051: trace: Use Create event type for local invoke

7d7551c

#1051: vrt-collection: Add doxygen documentation for invokable struct

3c6a390

#1051: trace: Fix incorrect trace name for invoke and remove localInv…

e1d4ad4

…oke function from trace module

JacobDomagala force-pushed the 1051-add-local-invoke-for-proxies branch from 3cfcf22 to e1d4ad4 Compare December 16, 2020 07:47

JacobDomagala merged commit 8acd899 into develop Dec 16, 2020

cz4rs mentioned this pull request Jun 15, 2021

move vt version number into a separate file #1467

Closed

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

1051: Add local invoke to proxies that trace and record LB #1132

1051: Add local invoke to proxies that trace and record LB #1132

JacobDomagala commented Oct 29, 2020 •

edited

Loading

codecov bot commented Oct 29, 2020 •

edited

Loading

JacobDomagala Nov 23, 2020

lifflander Nov 24, 2020

lifflander Nov 24, 2020

lifflander commented Nov 24, 2020

lifflander left a comment

lifflander Nov 24, 2020

lifflander Nov 24, 2020

lifflander Nov 24, 2020

JacobDomagala commented Nov 30, 2020

JacobDomagala commented Dec 7, 2020

lifflander commented Dec 16, 2020


		namespace vt { namespace vrt { namespace collection {

		template <typename ColT, typename IndexT, typename BaseProxyT>

1051: Add local invoke to proxies that trace and record LB #1132

1051: Add local invoke to proxies that trace and record LB #1132

Conversation

JacobDomagala commented Oct 29, 2020 • edited Loading

codecov bot commented Oct 29, 2020 • edited Loading

Codecov Report

JacobDomagala Nov 23, 2020

Choose a reason for hiding this comment

lifflander Nov 24, 2020

Choose a reason for hiding this comment

lifflander Nov 24, 2020

Choose a reason for hiding this comment

lifflander commented Nov 24, 2020

lifflander left a comment

Choose a reason for hiding this comment

lifflander Nov 24, 2020

Choose a reason for hiding this comment

lifflander Nov 24, 2020

Choose a reason for hiding this comment

lifflander Nov 24, 2020

Choose a reason for hiding this comment

JacobDomagala commented Nov 30, 2020

JacobDomagala commented Dec 7, 2020

lifflander commented Dec 16, 2020

JacobDomagala commented Oct 29, 2020 •

edited

Loading

codecov bot commented Oct 29, 2020 •

edited

Loading