Make few tasks #184

MBkkt · 2022-07-18T23:37:19Z

Purpose

Remove atomic counter in shared state (we can use our state machine to determine when we need to remove it). atomic counter increment/decrement have a big cost in some scenarios
Decrease size of Future shared state to make it allocation cheaper
Some refactoring
Few fixes
Fix macOS and Windows CI
Add gcc-12 to CI

Related Information

Design document:
Bench PR: ...

Testing

This change is a trivial rework or code cleanup without any test coverage.
This change is already covered by existing tests.
This PR adds tests that were used to verify all changes.
There are tests in an external testing repository: ...

coveralls · 2022-07-19T02:55:06Z

Pull Request Test Coverage Report for Build 2763811443

313 of 313 (100.0%) changed or added relevant lines in 41 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 100.0%

Totals
Change from base Build 2565529157:	0.0%
Covered Lines:	1361
Relevant Lines:	1361

💛 - Coveralls

codecov · 2022-07-19T02:55:14Z

Codecov Report

Merging #184 (3df3aa1) into main (1284ef5) will not change coverage.
The diff coverage is 100.00%.

❗ Current head 3df3aa1 differs from pull request most recent head 21b3036. Consider uploading reports for the commit 21b3036 to get more accurate results

@@            Coverage Diff            @@
##              main      #184   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           57        60    +3     
  Lines         1348      1361   +13     
=========================================
+ Hits          1348      1361   +13

Impacted Files	Coverage Δ
include/yaclib/algo/when_all.hpp	`100.00% <ø> (ø)`
include/yaclib/algo/when_any.hpp	`100.00% <ø> (ø)`
include/yaclib/async/detail/base_core.hpp	`100.00% <ø> (ø)`
include/yaclib/async/run.hpp	`100.00% <ø> (ø)`
include/yaclib/coroutine/detail/on_awaiter.hpp	`100.00% <ø> (ø)`
include/yaclib/util/detail/nope_counter.hpp	`100.00% <ø> (ø)`
include/yaclib/util/detail/shared_func.hpp	`100.00% <ø> (ø)`
include/yaclib/util/intrusive_ptr.hpp	`100.00% <ø> (ø)`
src/executor/inline.cpp	`100.00% <ø> (ø)`
src/executor/manual.cpp	`100.00% <ø> (ø)`
... and 54 more

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

* Remove atomic counter in shared state (we can use our state machine to determine when we need to remove it). atomic counter increment/decrement have a big cost in some scenarios * Decrease size of Future shared state to make it allocation cheaper * Some refactoring * Few fixes * Fix macOS and Windows CI * Add gcc-12 to CI

MBkkt · 2022-07-30T00:24:53Z

src/async/base_core.cpp

+  YACLIB_ASSERT(std::exchange(_caller, &kEmptyRef) != nullptr);
+  caller->DecRef();
+  // auto expected = _callback.load(std::memory_order_acquire);
+  // if (expected == kEmpty || (expected & kMask) == kWaitNope) {


Hmm I think it can be checked like ((expected & kMask) & ~kWaitNope) == 0?

I don't sure about will it better ot not.
Now we are always have 1 exchange + 1 load + optional(check load) cas

If here will be cas will be 2 load + 2 optional(check load) cas (1-2), I don't think it's definitely better so needs benchmark

I think on x86 current code will be better but for arm maybe 2 cas better, especially if compiler remove unnecessary loads

No, have here cas instead of exchange definitely bad idea because we need cycle for timed wait case

MBkkt · 2022-08-01T18:27:22Z

.github/workflows/windows.yml

-        uses: egor-tensin/setup-mingw@v2
-        with:
-          platform: x64
+      # TODO(MBkkt) https://github.com/msys2/setup-msys2?


Need to create issue

MBkkt · 2022-08-01T19:00:51Z

src/async/base_core.cpp

-  _executor->Submit(callback);
+#ifdef YACLIB_LOG_DEBUG
+BaseCore::~BaseCore() noexcept {
+  YACLIB_ASSERT(_caller == &kEmptyRef);


add more asserts

MBkkt · 2022-08-01T19:03:48Z

include/yaclib/async/detail/base_core.hpp

+#endif
+
+  yaclib_std::atomic_uint64_t _callback;
+  IRef* _caller{nullptr};


remove default initialize

MBkkt · 2022-08-01T19:04:31Z

src/async/base_core.cpp

-IExecutor* BaseCore::GetExecutor() const noexcept {
-  return _executor.Get();
+bool BaseCore::SetWait(IRef& callback, State state) noexcept {
+  // YACLIB_ASSERT(state == kWaitNope);


remove comment

MBkkt · 2022-08-01T19:06:33Z

src/async/base_core.cpp

 }

-IExecutor* BaseCore::GetExecutor() const noexcept {
-  return _executor.Get();
+bool BaseCore::SetWait(IRef& callback, State state) noexcept {


maybe we can remove virtual call for waiters

MBkkt · 2022-08-01T19:23:23Z

src/async/base_core.cpp

+    case kHereCall: {
+      auto* callback = reinterpret_cast<IRef*>(expected & ~kMask);
+      YACLIB_ASSERT(callback != nullptr);
+      Submit(static_cast<BaseCore&>(*callback), state);


InlineCore&

MBkkt · 2022-08-01T19:36:41Z

include/yaclib/util/detail/safe_call.hpp

@@ -31,7 +33,7 @@ class SafeCall {
  }

 private:
-  Store _func;
+  [[no_unique_address]] Store _func;


maybe make macro to prevent unknown attribute warning

MBkkt · 2022-08-01T19:40:58Z

include/yaclib/executor/detail/unique_job.hpp

 template <typename Func>
 Job* MakeUniqueJob(Func&& f) {
-  return new UniqueJob<Func>{std::forward<Func>(f)};
+  return new UniqueJob<decltype(std::forward<Func>(f))>{std::forward<Func>(f)};


MBkkt · 2022-08-01T19:43:41Z

include/yaclib/async/task.hpp

@@ -0,0 +1,95 @@
+#pragma once


remove or implement

MBkkt requested review from myannyax and kononovk as code owners July 18, 2022 23:37

MBkkt self-assigned this Jul 19, 2022

MBkkt assigned MBkkt and unassigned MBkkt Jul 29, 2022

MBkkt force-pushed the add-task2-rebase branch 5 times, most recently from 305e030 to 1a1def5 Compare July 29, 2022 20:54

MBkkt added devops DevOps tasks performance Improve performance refactoring Refactoring labels Jul 29, 2022

MBkkt assigned MBkkt and unassigned MBkkt Jul 29, 2022

MBkkt force-pushed the add-task2-rebase branch 2 times, most recently from fa8057a to a3a5359 Compare July 29, 2022 23:55

MBkkt force-pushed the add-task2-rebase branch 2 times, most recently from ea5782a to 21b3036 Compare July 30, 2022 00:16

MBkkt commented Jul 30, 2022

View reviewed changes

MBkkt assigned MBkkt and unassigned MBkkt Jul 30, 2022

MBkkt merged commit 16d8857 into main Jul 30, 2022

MBkkt commented Aug 1, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make few tasks #184

Make few tasks #184

MBkkt commented Jul 18, 2022 •

edited

Loading

coveralls commented Jul 19, 2022 •

edited

Loading

codecov bot commented Jul 19, 2022 •

edited

Loading

MBkkt Jul 30, 2022 •

edited

Loading

MBkkt Jul 31, 2022

MBkkt Aug 1, 2022

MBkkt Aug 1, 2022

MBkkt Aug 1, 2022

MBkkt Aug 1, 2022

MBkkt Aug 1, 2022

MBkkt Aug 1, 2022

MBkkt Aug 1, 2022

MBkkt Aug 1, 2022

MBkkt Aug 1, 2022

Make few tasks #184

Make few tasks #184

Conversation

MBkkt commented Jul 18, 2022 • edited Loading

Purpose

Related Information

Testing

coveralls commented Jul 19, 2022 • edited Loading

Pull Request Test Coverage Report for Build 2763811443

💛 - Coveralls

codecov bot commented Jul 19, 2022 • edited Loading

Codecov Report

MBkkt Jul 30, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MBkkt commented Jul 18, 2022 •

edited

Loading

coveralls commented Jul 19, 2022 •

edited

Loading

codecov bot commented Jul 19, 2022 •

edited

Loading

MBkkt Jul 30, 2022 •

edited

Loading