Optimizations on LCI parcelport: merge small messages; remove sender mutex lock. #6105

JiakunYan · 2022-12-15T22:45:50Z

Changes

Adapt to LCI v1.7.2 CMakeList.txt changes: LCI_PM_BACKEND renamed to LCI_PM_BACKEND_DEFAULT; new object lci-ucx (the ucx registration cache) to install and export; renamed include directory.
Send header, data (non-zero-copy chunks), and transmission chunks in a single message if possible
Reduce the thread contention on the sender side: replace sync with completion queue: remove mutex lock on sender_connection.

hkaiser · 2022-12-16T15:24:59Z

@JiakunYan could you please take care of the inspect, clang-format and cmake-format complaints?

JiakunYan · 2022-12-16T16:13:44Z

@hkaiser Done. I am looking at the failed tests. OpenMPI links to UCX and LCI also links to part of the UCX (the registration cache). It seems there are some conflicts. It is weird I didn't have this issue on my system.

libs/full/parcelport_lci/include/hpx/parcelport_lci/sender_connection.hpp

libs/core/lci_base/include/hpx/lci_base/lci_environment.hpp

StellarBot · 2022-12-17T23:38:33Z

Performance test report

HPX Performance

Comparison

BENCHMARK	FORK_JOIN_EXECUTOR	PARALLEL_EXECUTOR	SCHEDULER_EXECUTOR
For Each	(=)	(=)	(=)

Info

Property	Before	After
HPX Datetime	2022-05-31T12:57:29+00:00	2022-12-17T23:16:20+00:00
HPX Commit	`d5655f8`	`1dc8f89`
Envfile
Compiler	/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0	/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0
Hostname	nid01193	nid01538
Datetime	2022-05-31T15:13:01.357969+02:00	2022-12-18T00:37:29.848302+01:00
Clustername	daint	daint

Comparison

BENCHMARK	NO-EXECUTOR
Future Overhead - Create Thread Hierarchical - Latch	++

Info

Property	Before	After
HPX Datetime	2022-05-31T12:57:29+00:00	2022-12-17T23:16:20+00:00
HPX Commit	`d5655f8`	`1dc8f89`
Envfile
Compiler	/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0	/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0
Hostname	nid01193	nid01538
Datetime	2022-05-31T15:13:18.026239+02:00	2022-12-18T00:37:45.681060+01:00
Clustername	daint	daint

Comparison

BENCHMARK	FORK_JOIN_EXECUTOR_DEFAULT_FORK_JOIN_POLICY_ALLOCATOR	PARALLEL_EXECUTOR_DEFAULT_PARALLEL_POLICY_ALLOCATOR	SCHEDULER_EXECUTOR_DEFAULT_SCHEDULER_EXECUTOR_ALLOCATOR
Stream Benchmark - Add	(=)	(=)	(=)
Stream Benchmark - Scale	(=)	(=)	(=)
Stream Benchmark - Triad	-	(=)	(=)
Stream Benchmark - Copy	(=)	(=)	(=)

Info

Property	Before	After
HPX Datetime	2021-11-10T19:14:21+00:00	2022-12-17T23:16:20+00:00
HPX Commit	`71d8dbe`	`1dc8f89`
Envfile
Compiler	/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0	/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0
Hostname	nid00120	nid01538
Datetime	2021-11-10T20:28:18.266961+01:00	2022-12-18T00:38:01.178865+01:00
Clustername	daint	daint

Explanation of Symbols

Symbol	MEANING
=	No performance change (confidence interval within ±1%)
(=)	Probably no performance change (confidence interval within ±2%)
(+)/(-)	Very small performance improvement/degradation (≤1%)
+/-	Small performance improvement/degradation (≤5%)
++/--	Large performance improvement/degradation (≤10%)
+++/---	Very large performance improvement/degradation (>10%)
?	Probably no change, but quite large uncertainty (confidence interval with ±5%)
??	Unclear result, very large uncertainty (±10%)
???	Something unexpected…

JiakunYan · 2022-12-19T03:58:05Z

@hkaiser would it be better if I squash these commits into one?

hkaiser · 2022-12-20T13:57:40Z

@hkaiser would it be better if I squash these commits into one?

@JiakunYan please do as you prefer.

JiakunYan · 2022-12-22T18:44:41Z

@hkaiser Done. I think this PR is ready to be merged.

hkaiser · 2023-01-05T15:42:15Z

@JiakunYan would you mind having a look at https://app.codacy.com/gh/STEllAR-GROUP/hpx/pullRequest?prid=10784819?

JiakunYan · 2023-01-08T21:06:56Z

@JiakunYan would you mind having a look at https://app.codacy.com/gh/STEllAR-GROUP/hpx/pullRequest?prid=10784819?

Fixed!

StellarBot · 2023-01-09T20:00:09Z

Performance test report

HPX Performance

Comparison

BENCHMARK	FORK_JOIN_EXECUTOR	PARALLEL_EXECUTOR	SCHEDULER_EXECUTOR
For Each	(=)	+	(=)

Info

Property	Before	After
HPX Datetime	2022-05-31T12:57:29+00:00	2023-01-09T19:44:45+00:00
HPX Commit	`d5655f8`	`95fda35`
Hostname	nid01193	nid00025
Datetime	2022-05-31T15:13:01.357969+02:00	2023-01-09T20:59:02.165290+01:00
Compiler	/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0	/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0
Envfile
Clustername	daint	daint

Comparison

BENCHMARK	NO-EXECUTOR
Future Overhead - Create Thread Hierarchical - Latch	++

Info

Property	Before	After
HPX Datetime	2022-05-31T12:57:29+00:00	2023-01-09T19:44:45+00:00
HPX Commit	`d5655f8`	`95fda35`
Hostname	nid01193	nid00025
Datetime	2022-05-31T15:13:18.026239+02:00	2023-01-09T20:59:18.053538+01:00
Compiler	/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0	/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0
Envfile
Clustername	daint	daint

Comparison

BENCHMARK	FORK_JOIN_EXECUTOR_DEFAULT_FORK_JOIN_POLICY_ALLOCATOR	PARALLEL_EXECUTOR_DEFAULT_PARALLEL_POLICY_ALLOCATOR	SCHEDULER_EXECUTOR_DEFAULT_SCHEDULER_EXECUTOR_ALLOCATOR
Stream Benchmark - Add	-	+	(=)
Stream Benchmark - Scale	(=)	+++	(=)
Stream Benchmark - Triad	(=)	++	-
Stream Benchmark - Copy	(=)	++	(=)

Info

Property	Before	After
HPX Datetime	2021-11-10T19:14:21+00:00	2023-01-09T19:44:45+00:00
HPX Commit	`71d8dbe`	`95fda35`
Hostname	nid00120	nid00025
Datetime	2021-11-10T20:28:18.266961+01:00	2023-01-09T20:59:33.010934+01:00
Compiler	/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0	/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0
Envfile
Clustername	daint	daint

Explanation of Symbols

Symbol	MEANING
=	No performance change (confidence interval within ±1%)
(=)	Probably no performance change (confidence interval within ±2%)
(+)/(-)	Very small performance improvement/degradation (≤1%)
+/-	Small performance improvement/degradation (≤5%)
++/--	Large performance improvement/degradation (≤10%)
+++/---	Very large performance improvement/degradation (>10%)
?	Probably no change, but quite large uncertainty (confidence interval with ±5%)
??	Unclear result, very large uncertainty (±10%)
???	Something unexpected…

hkaiser

LGTM, thanks!

hkaiser · 2023-01-10T14:52:48Z

This PR contains the changes from #6057, where I have requested changes. Once that has been merged, this PR needs to be rebased.

StellarBot · 2023-01-23T06:39:05Z

Performance test report

HPX Performance

Comparison

BENCHMARK	FORK_JOIN_EXECUTOR	PARALLEL_EXECUTOR	SCHEDULER_EXECUTOR
For Each	(=)	(=)	(=)

Info

Property	Before	After
HPX Commit	`d482723`	`81247c2`
HPX Datetime	2023-01-12T14:41:15+00:00	2023-01-23T06:23:31+00:00
Compiler	/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0	/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0
Hostname	nid00267	nid01351
Datetime	2023-01-12T16:07:32.649261+01:00	2023-01-23T07:37:44.796591+01:00
Clustername	daint	daint
Envfile

Comparison

BENCHMARK	NO-EXECUTOR
Future Overhead - Create Thread Hierarchical - Latch	---

Info

Property	Before	After
HPX Commit	`da01e78`	`81247c2`
HPX Datetime	2023-01-18T08:41:20+00:00	2023-01-23T06:23:31+00:00
Compiler	/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0	/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0
Hostname	nid00005	nid01351
Datetime	2023-01-18T09:57:57.669847+01:00	2023-01-23T07:38:05.440110+01:00
Clustername	daint	daint
Envfile

Comparison

BENCHMARK	FORK_JOIN_EXECUTOR_DEFAULT_FORK_JOIN_POLICY_ALLOCATOR	PARALLEL_EXECUTOR_DEFAULT_PARALLEL_POLICY_ALLOCATOR	SCHEDULER_EXECUTOR_DEFAULT_SCHEDULER_EXECUTOR_ALLOCATOR
Stream Benchmark - Add	+	(=)	(=)
Stream Benchmark - Scale	(=)	(=)	(=)
Stream Benchmark - Triad	(=)	(=)	(=)
Stream Benchmark - Copy	(=)	(=)	(=)

Info

Property	Before	After
HPX Commit	`52952d8`	`81247c2`
HPX Datetime	2023-01-17T08:25:42+00:00	2023-01-23T06:23:31+00:00
Compiler	/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0	/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0
Hostname	nid00005	nid01351
Datetime	2023-01-17T09:44:28.090718+01:00	2023-01-23T07:38:20.921552+01:00
Clustername	daint	daint
Envfile

Explanation of Symbols

Symbol	MEANING
=	No performance change (confidence interval within ±1%)
(=)	Probably no performance change (confidence interval within ±2%)
(+)/(-)	Very small performance improvement/degradation (≤1%)
+/-	Small performance improvement/degradation (≤5%)
++/--	Large performance improvement/degradation (≤10%)
+++/---	Very large performance improvement/degradation (>10%)
?	Probably no change, but quite large uncertainty (confidence interval with ±5%)
??	Unclear result, very large uncertainty (±10%)
???	Something unexpected…

…mutex lock - send header, data, tchunk in a single message if possible - replace sync with completion queue: remove lock on sender_connection - adapt to LCI v1.7.2 cmake change - Use unique_ptr for prg_thread_p

StellarBot · 2023-01-29T21:24:22Z

Performance test report

HPX Performance

Comparison

BENCHMARK	FORK_JOIN_EXECUTOR	PARALLEL_EXECUTOR	SCHEDULER_EXECUTOR
For Each	=	(=)	(=)

Info

Property	Before	After
HPX Commit	`d482723`	`633bf61`
HPX Datetime	2023-01-12T14:41:15+00:00	2023-01-29T20:33:09+00:00
Datetime	2023-01-12T16:07:32.649261+01:00	2023-01-29T22:23:19.701536+01:00
Hostname	nid00267	nid00446
Compiler	/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0	/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0
Clustername	daint	daint
Envfile

Comparison

BENCHMARK	NO-EXECUTOR
Future Overhead - Create Thread Hierarchical - Latch	--

Info

Property	Before	After
HPX Commit	`da01e78`	`633bf61`
HPX Datetime	2023-01-18T08:41:20+00:00	2023-01-29T20:33:09+00:00
Datetime	2023-01-18T09:57:57.669847+01:00	2023-01-29T22:23:39.771520+01:00
Hostname	nid00005	nid00446
Compiler	/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0	/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0
Clustername	daint	daint
Envfile

Comparison

BENCHMARK	FORK_JOIN_EXECUTOR_DEFAULT_FORK_JOIN_POLICY_ALLOCATOR	PARALLEL_EXECUTOR_DEFAULT_PARALLEL_POLICY_ALLOCATOR	SCHEDULER_EXECUTOR_DEFAULT_SCHEDULER_EXECUTOR_ALLOCATOR
Stream Benchmark - Add	(=)	(=)	(=)
Stream Benchmark - Scale	(=)	(=)	(=)
Stream Benchmark - Triad	(=)	(=)	(=)
Stream Benchmark - Copy	(=)	(=)	=

Info

Property	Before	After
HPX Commit	`52952d8`	`633bf61`
HPX Datetime	2023-01-17T08:25:42+00:00	2023-01-29T20:33:09+00:00
Datetime	2023-01-17T09:44:28.090718+01:00	2023-01-29T22:23:55.014954+01:00
Hostname	nid00005	nid00446
Compiler	/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0	/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0
Clustername	daint	daint
Envfile

Explanation of Symbols

Symbol	MEANING
=	No performance change (confidence interval within ±1%)
(=)	Probably no performance change (confidence interval within ±2%)
(+)/(-)	Very small performance improvement/degradation (≤1%)
+/-	Small performance improvement/degradation (≤5%)
++/--	Large performance improvement/degradation (≤10%)
+++/---	Very large performance improvement/degradation (>10%)
?	Probably no change, but quite large uncertainty (confidence interval with ±5%)
??	Unclear result, very large uncertainty (±10%)
???	Something unexpected…

StellarBot · 2023-01-30T23:16:45Z

Performance test report

HPX Performance

Comparison

BENCHMARK	FORK_JOIN_EXECUTOR	PARALLEL_EXECUTOR	SCHEDULER_EXECUTOR
For Each	(=)	(=)	(=)

Info

Property	Before	After
HPX Datetime	2023-01-12T14:41:15+00:00	2023-01-30T23:04:43+00:00
HPX Commit	`d482723`	`6f8c7b6`
Hostname	nid00267	nid00025
Clustername	daint	daint
Envfile
Datetime	2023-01-12T16:07:32.649261+01:00	2023-01-31T00:15:38.520013+01:00
Compiler	/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0	/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0

Comparison

BENCHMARK	NO-EXECUTOR
Future Overhead - Create Thread Hierarchical - Latch	??

Info

Property	Before	After
HPX Datetime	2023-01-18T08:41:20+00:00	2023-01-30T23:04:43+00:00
HPX Commit	`da01e78`	`6f8c7b6`
Hostname	nid00005	nid00025
Clustername	daint	daint
Envfile
Datetime	2023-01-18T09:57:57.669847+01:00	2023-01-31T00:15:59.154271+01:00
Compiler	/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0	/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0

Comparison

BENCHMARK	FORK_JOIN_EXECUTOR_DEFAULT_FORK_JOIN_POLICY_ALLOCATOR	PARALLEL_EXECUTOR_DEFAULT_PARALLEL_POLICY_ALLOCATOR	SCHEDULER_EXECUTOR_DEFAULT_SCHEDULER_EXECUTOR_ALLOCATOR
Stream Benchmark - Add	(=)	(=)	(=)
Stream Benchmark - Scale	(=)	(=)	(=)
Stream Benchmark - Triad	(=)	(=)	(=)
Stream Benchmark - Copy	(=)	(=)	(=)

Info

Property	Before	After
HPX Datetime	2023-01-17T08:25:42+00:00	2023-01-30T23:04:43+00:00
HPX Commit	`52952d8`	`6f8c7b6`
Hostname	nid00005	nid00025
Clustername	daint	daint
Envfile
Datetime	2023-01-17T09:44:28.090718+01:00	2023-01-31T00:16:14.356514+01:00
Compiler	/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0	/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0

Explanation of Symbols

Symbol	MEANING
=	No performance change (confidence interval within ±1%)
(=)	Probably no performance change (confidence interval within ±2%)
(+)/(-)	Very small performance improvement/degradation (≤1%)
+/-	Small performance improvement/degradation (≤5%)
++/--	Large performance improvement/degradation (≤10%)
+++/---	Very large performance improvement/degradation (>10%)
?	Probably no change, but quite large uncertainty (confidence interval with ±5%)
??	Unclear result, very large uncertainty (±10%)
???	Something unexpected…

JiakunYan · 2023-01-31T23:01:30Z

@hkaiser This test failed: tests.unit.modules.runtime_components.distributed.tcp.migrate_polymorphic_component in 6105-clang-11-debug

Do you think it is related to this PR?

hkaiser · 2023-02-02T19:40:08Z

@hkaiser This test failed: tests.unit.modules.runtime_components.distributed.tcp.migrate_polymorphic_component in 6105-clang-11-debug

Do you think it is related to this PR?

No, this is a known issue and unrelated to your work.

JiakunYan · 2023-02-03T22:39:21Z

No, this is a known issue and unrelated to your work.

Then I think this PR is ready to merge.

JiakunYan · 2023-02-08T23:41:53Z

@hkaiser Is there anything else I should do before we merge this PR?

hkaiser

LGTM, thanks!

hkaiser · 2023-02-09T21:30:01Z

bors merge

6105: Optimizations on LCI parcelport: merge small messages; remove sender mutex lock. r=hkaiser a=JiakunYan ## Changes - Adapt to LCI v1.7.2 CMakeList.txt changes: LCI_PM_BACKEND renamed to LCI_PM_BACKEND_DEFAULT; new object lci-ucx (the ucx registration cache) to install and export; renamed include directory. - Send header, data (non-zero-copy chunks), and transmission chunks in a single message if possible - Reduce the thread contention on the sender side: replace sync with completion queue: remove mutex lock on sender_connection. Co-authored-by: Jiakun Yan <jiakunyan1998@gmail.com>

bors · 2023-02-09T22:17:51Z

This PR was included in a batch that successfully built, but then failed to merge into master (it was a non-fast-forward update). It will be automatically retried.

6105: Optimizations on LCI parcelport: merge small messages; remove sender mutex lock. r=hkaiser a=JiakunYan ## Changes - Adapt to LCI v1.7.2 CMakeList.txt changes: LCI_PM_BACKEND renamed to LCI_PM_BACKEND_DEFAULT; new object lci-ucx (the ucx registration cache) to install and export; renamed include directory. - Send header, data (non-zero-copy chunks), and transmission chunks in a single message if possible - Reduce the thread contention on the sender side: replace sync with completion queue: remove mutex lock on sender_connection. Co-authored-by: Jiakun Yan <jiakunyan1998@gmail.com>

bors · 2023-02-09T23:30:46Z

Timed out.

JiakunYan requested review from aurianer, msimberg, biddisco and hkaiser as code owners December 15, 2022 22:45

hkaiser added type: optimization type: enhancement category: parcel transport labels Dec 16, 2022

hkaiser added this to the 1.9.0 milestone Dec 16, 2022

hkaiser reviewed Dec 17, 2022

View reviewed changes

libs/full/parcelport_lci/include/hpx/parcelport_lci/sender_connection.hpp Show resolved Hide resolved

libs/core/lci_base/include/hpx/lci_base/lci_environment.hpp Outdated Show resolved Hide resolved

JiakunYan force-pushed the opt branch from b0e3931 to f500a60 Compare December 18, 2022 06:17

JiakunYan force-pushed the opt branch from 7d87cb4 to cc1a27e Compare December 21, 2022 02:50

JiakunYan force-pushed the opt branch from cc1a27e to f3be8ff Compare January 7, 2023 22:17

JiakunYan force-pushed the opt branch from f3be8ff to 71bd7e8 Compare January 9, 2023 19:44

hkaiser previously approved these changes Jan 10, 2023

View reviewed changes

JiakunYan dismissed hkaiser’s stale review via b5c00a9 January 23, 2023 06:22

JiakunYan force-pushed the opt branch from 71bd7e8 to b5c00a9 Compare January 23, 2023 06:22

Optimizations on LCI parcelport: merge small messages; remove sender …

6c99e3b

…mutex lock - send header, data, tchunk in a single message if possible - replace sync with completion queue: remove lock on sender_connection - adapt to LCI v1.7.2 cmake change - Use unique_ptr for prg_thread_p

JiakunYan force-pushed the opt branch from a68bed1 to 6c99e3b Compare January 29, 2023 20:33

try to fix the assertion error

bb86290

hkaiser approved these changes Feb 9, 2023

View reviewed changes

JiakunYan mentioned this pull request Feb 9, 2023

LCI parcelport: bypass the parcel queue and connection cache. #6169

Merged

hkaiser merged commit f11a351 into STEllAR-GROUP:master Feb 10, 2023

Optimizations on LCI parcelport: merge small messages; remove sender mutex lock. #6105

Optimizations on LCI parcelport: merge small messages; remove sender mutex lock. #6105

Conversation

JiakunYan commented Dec 15, 2022 • edited Loading

Changes

hkaiser commented Dec 16, 2022

JiakunYan commented Dec 16, 2022

StellarBot commented Dec 17, 2022

HPX Performance

Comparison

Info

Comparison

Info

Comparison

Info

Explanation of Symbols

JiakunYan commented Dec 19, 2022

hkaiser commented Dec 20, 2022

JiakunYan commented Dec 22, 2022

hkaiser commented Jan 5, 2023

JiakunYan commented Jan 8, 2023

StellarBot commented Jan 9, 2023

HPX Performance

Comparison

Info

Comparison

Info

Comparison

Info

Explanation of Symbols

hkaiser left a comment

Choose a reason for hiding this comment

hkaiser commented Jan 10, 2023

StellarBot commented Jan 23, 2023

HPX Performance

Comparison

Info

Comparison

Info

Comparison

Info

Explanation of Symbols

StellarBot commented Jan 29, 2023

HPX Performance

Comparison

Info

Comparison

Info

Comparison

Info

Explanation of Symbols

StellarBot commented Jan 30, 2023

HPX Performance

Comparison

Info

Comparison

Info

Comparison

Info

Explanation of Symbols

JiakunYan commented Jan 31, 2023

hkaiser commented Feb 2, 2023

JiakunYan commented Feb 3, 2023

JiakunYan commented Feb 8, 2023

hkaiser left a comment

Choose a reason for hiding this comment

hkaiser commented Feb 9, 2023

bors bot commented Feb 9, 2023

bors bot commented Feb 9, 2023

JiakunYan commented Dec 15, 2022 •

edited

Loading