2037 - Remove workers from vt #2058

thearusable · 2023-01-02T13:34:09Z

Closes #2037

src/vt/pool/header/pool_header.cc

src/vt/pool/pool.cc

.github/workflows/macosx-clang-mpich.yml

lifflander · 2023-01-03T18:17:17Z

There should also be some removals from documentation.

thearusable · 2023-01-05T14:31:15Z

@lifflander Should I also remove the fcontext?

PhilMiller

Definitely don't remove fcontext. We're using that for real application work, and actively developing and improving it

codecov · 2023-01-09T14:54:46Z

Codecov Report

Merging #2058 (708a084) into develop (9089be5) will increase coverage by 0.31%.
The diff coverage is 81.94%.

❗ Current head 708a084 differs from pull request most recent head 26333a5. Consider uploading reports for the commit 26333a5 to get more accurate results

@@             Coverage Diff             @@
##           develop    #2058      +/-   ##
===========================================
+ Coverage    84.47%   84.78%   +0.31%     
===========================================
  Files          730      723       -7     
  Lines        25842    25647     -195     
===========================================
- Hits         21829    21746      -83     
+ Misses        4013     3901     -112

Impacted Files	Coverage Δ
src/vt/collective/collective_ops.cc	`92.04% <0.00%> (+1.03%)`	⬆️
src/vt/context/context.cc	`100.00% <ø> (ø)`
src/vt/context/context.h	`100.00% <ø> (+8.33%)`	⬆️
src/vt/context/context_attorney.cc	`100.00% <ø> (+66.66%)`	⬆️
src/vt/pool/header/pool_header.cc	`100.00% <ø> (ø)`
src/vt/pool/pool.cc	`92.40% <ø> (+4.90%)`	⬆️
src/vt/runtime/runtime.h	`100.00% <ø> (ø)`
src/vt/scheduler/scheduler.cc	`68.71% <ø> (ø)`
src/vt/standalone/vt_main.h	`100.00% <ø> (ø)`
src/vt/utils/atomic/null_atomic.h	`80.00% <ø> (ø)`
... and 37 more

cz4rs · 2023-01-09T21:02:08Z

The following tests FAILED:
	395 - vt:TestPreconfig.test_vt_assert_no_mpi (SEGFAULT)

I think this might be an independent failure caused by a newer intel/oneapi image available. I'll try to reproduce this on develop and create a separate issue if needed.

cmake/threading_config.cmake

docs/md/context.md

docs/md/vt.md

src/vt/configs/debug/debug_masterconfig.h

src/vt/runtime/runtime.cc

src/vt/runtime/runtime_get.cc

src/vt/utils/container/process_ready_buffer.h

lifflander

Looks good to me!

lifflander · 2023-01-12T23:22:50Z

The failing pipelines seem to have nothing to do with this PR.

PhilMiller · 2023-01-13T00:41:27Z

The commit signatures will need to be fixed

cmake/load_threading_package.cmake

lifflander · 2023-01-13T00:51:09Z

The commit signatures will need to be fixed

There's no problem with them. The github action is being very unstable.

src/vt/utils/atomic/atomic.h

src/vt/utils/mutex/mutex.h

tests/unit/atomic/test_atomic.cc

PhilMiller

Other than maybe some left-over bits that can also be deleted, this looks good to me.

cz4rs · 2023-01-13T19:04:26Z

The following tests FAILED:
	395 - vt:TestPreconfig.test_vt_assert_no_mpi (SEGFAULT)
I think this might be an independent failure caused by a newer intel/oneapi image available. I'll try to reproduce this on develop and create a separate issue if needed.

This runs fine on my machine with IntelLLVM 2023.0.0

-- The C compiler identification is IntelLLVM 2023.0.0
-- The CXX compiler identification is IntelLLVM 2023.0.0

vt  build  ctest -R TestPreconfig.test_vt_assert_no_mpi
Test project /home/cz4rs/code/vt/build
    Start 995: vt:TestPreconfig.test_vt_assert_no_mpi
1/1 Test #995: vt:TestPreconfig.test_vt_assert_no_mpi ...   Passed    0.00 sec

100% tests passed, 0 tests failed out of 1

I'll try running it inside container 🤷

PhilMiller · 2023-01-14T01:15:49Z

cmake/load_threading_package.cmake

@@ -20,11 +10,11 @@ if(vt_fcontext_enabled)
    STATUS
    "Using fcontext for worker threading"
  )
-  config_for_fcontext()
+  set(vt_fcontext_enabled "1")


This can go away too, can't it? if (X) set(X)?

Ah yes! removed.

src/vt/utils/mutex/mutex.h

thearusable self-assigned this Jan 2, 2023

lifflander reviewed Jan 3, 2023

View reviewed changes

src/vt/pool/header/pool_header.cc Outdated Show resolved Hide resolved

lifflander reviewed Jan 3, 2023

View reviewed changes

src/vt/pool/pool.cc Outdated Show resolved Hide resolved

lifflander reviewed Jan 3, 2023

View reviewed changes

.github/workflows/macosx-clang-mpich.yml Outdated Show resolved Hide resolved

thearusable force-pushed the 2037-remove-workers branch 2 times, most recently from d4c6454 to 95d3d8a Compare January 5, 2023 14:27

thearusable force-pushed the 2037-remove-workers branch from 95d3d8a to 67c97ad Compare January 5, 2023 15:20

PhilMiller requested changes Jan 5, 2023

View reviewed changes

thearusable force-pushed the 2037-remove-workers branch 3 times, most recently from 2ac952b to 641ddf9 Compare January 6, 2023 14:00

thearusable marked this pull request as ready for review January 6, 2023 15:36

thearusable force-pushed the 2037-remove-workers branch 3 times, most recently from a4280c1 to 77bcb99 Compare January 9, 2023 14:42

cz4rs reviewed Jan 9, 2023

View reviewed changes

cmake/threading_config.cmake Outdated Show resolved Hide resolved

cz4rs reviewed Jan 9, 2023

View reviewed changes

docs/md/context.md Outdated Show resolved Hide resolved

cz4rs reviewed Jan 9, 2023

View reviewed changes

docs/md/vt.md Outdated Show resolved Hide resolved

thearusable force-pushed the 2037-remove-workers branch from 77bcb99 to 149d9bd Compare January 10, 2023 13:45

thearusable requested review from lifflander, cz4rs, PhilMiller, JacobDomagala, nlslatt, nmm0 and stmcgovern January 10, 2023 17:16

lifflander requested changes Jan 10, 2023

View reviewed changes

src/vt/configs/debug/debug_masterconfig.h Outdated Show resolved Hide resolved

src/vt/runtime/runtime.cc Outdated Show resolved Hide resolved

src/vt/runtime/runtime_get.cc Outdated Show resolved Hide resolved

src/vt/utils/container/process_ready_buffer.h Outdated Show resolved Hide resolved

thearusable force-pushed the 2037-remove-workers branch 2 times, most recently from f99fcd1 to 708a084 Compare January 12, 2023 10:43

lifflander approved these changes Jan 12, 2023

View reviewed changes

PhilMiller reviewed Jan 13, 2023

View reviewed changes

cmake/load_threading_package.cmake Outdated Show resolved Hide resolved

PhilMiller reviewed Jan 13, 2023

View reviewed changes

src/vt/utils/atomic/atomic.h Outdated Show resolved Hide resolved

PhilMiller reviewed Jan 13, 2023

View reviewed changes

src/vt/utils/mutex/mutex.h Outdated Show resolved Hide resolved

PhilMiller reviewed Jan 13, 2023

View reviewed changes

tests/unit/atomic/test_atomic.cc Show resolved Hide resolved

PhilMiller approved these changes Jan 13, 2023

View reviewed changes

PhilMiller reviewed Jan 14, 2023

View reviewed changes

src/vt/utils/mutex/mutex.h Outdated Show resolved Hide resolved

thearusable added 12 commits January 16, 2023 12:39

#2037: Remove workers

4beb0c9

#2037: Remove references to stdthread

81c4a5a

#2037: Remove VT_USE_STD_THREAD

9ca2def

#2037: Remove TLS and worker from Context

7e71ac6

#2037: Remove no_workers

a697f1d

#2037: Remove openmp

8fe8d3a

#2037: Update CMake files and documentation

f3ba709

#2037: Remove unnecessary macros after workers deletion

d2b1a3d

#2037: Rework handling of messages buffer in context_vrtinfo

3f4f681

#2037: Remove CMake functions for enabling / disabling fcontext

ad4e73e

#2037: Remove null_atomic

6deb87e

#2037: Remove null mutex

26333a5

thearusable force-pushed the 2037-remove-workers branch from 319677c to 26333a5 Compare January 16, 2023 11:39

cz4rs mentioned this pull request Jan 17, 2023

TestPreconfig.test_vt_assert_no_mpi segfaults in icpx build #2068

Closed

lifflander merged commit c93daf8 into develop Jan 17, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

2037 - Remove workers from vt #2058

2037 - Remove workers from vt #2058

thearusable commented Jan 2, 2023

lifflander commented Jan 3, 2023

thearusable commented Jan 5, 2023

PhilMiller left a comment

codecov bot commented Jan 9, 2023 •

edited

Loading

cz4rs commented Jan 9, 2023

lifflander left a comment

lifflander commented Jan 12, 2023

PhilMiller commented Jan 13, 2023

lifflander commented Jan 13, 2023

PhilMiller left a comment

cz4rs commented Jan 13, 2023

PhilMiller Jan 14, 2023

thearusable Jan 16, 2023

2037 - Remove workers from vt #2058

2037 - Remove workers from vt #2058

Conversation

thearusable commented Jan 2, 2023

lifflander commented Jan 3, 2023

thearusable commented Jan 5, 2023

PhilMiller left a comment

Choose a reason for hiding this comment

codecov bot commented Jan 9, 2023 • edited Loading

Codecov Report

cz4rs commented Jan 9, 2023

lifflander left a comment

Choose a reason for hiding this comment

lifflander commented Jan 12, 2023

PhilMiller commented Jan 13, 2023

lifflander commented Jan 13, 2023

PhilMiller left a comment

Choose a reason for hiding this comment

cz4rs commented Jan 13, 2023

PhilMiller Jan 14, 2023

Choose a reason for hiding this comment

thearusable Jan 16, 2023

Choose a reason for hiding this comment

codecov bot commented Jan 9, 2023 •

edited

Loading