Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added optional scheduler blocking #376

Merged
merged 7 commits into from
Jul 18, 2024
Merged

added optional scheduler blocking #376

merged 7 commits into from
Jul 18, 2024

Conversation

RalphSteinhagen
Copy link
Member

@RalphSteinhagen RalphSteinhagen commented Jul 10, 2024

... in case there is no data to be processed and/or no IO activity.

  • added missing processBulk(..) user return status handling
  • changed ExecutionPolicy to 'enum class' (type safety) -> needed some adjustments in other unrelated unit-tests
  • added msg-to-stream processing ratio
  • simplified SchedulerBase and streamlined single- and multi-threaded handling
  • added provisions for runtime graph modifications

qa_PerformanceMonitor benchmark (GCC14, no Tags, 2" runtime) before:

Performance at 2024-07-10T07:43:10.000387, #30 dT:1.0002027 s, rate:435 MS/s, memory_resident:416 Mb
Performance at 2024-07-10T07:43:11.000389, #31 dT:1.0001781 s, rate:433 MS/s, memory_resident:416 Mb
Performance at 2024-07-10T07:43:12.000391, #32 dT:1.0002033 s, rate:426 MS/s, memory_resident:416 Mb
Performance at 2024-07-10T07:43:13.000394, #33 dT:1.0000606 s, rate:433 MS/s, memory_resident:416 Mb
Performance at 2024-07-10T07:43:14.000396, #34 dT:1.0000408 s, rate:441 MS/s, memory_resident:416 Mb

after (this PR);

Performance at 2024-07-11T00:06:38.000761, #4 dT:1.0002002 s, rate:442 MS/s, memory_resident:416 Mb
Performance at 2024-07-11T00:06:39.000764, #5 dT:1.0002015 s, rate:446 MS/s, memory_resident:416 Mb
Performance at 2024-07-11T00:06:40.000766, #6 dT:1.0001551 s, rate:431 MS/s, memory_resident:416 Mb
Performance at 2024-07-11T00:06:41.000768, #7 dT:1.0001084 s, rate:444 MS/s, memory_resident:416 Mb
Performance at 2024-07-11T00:06:42.000771, #8 dT:1.0000186 s, rate:439 MS/s, memory_resident:416 Mb

There have been some performance gains but the 'return status handling' added some overhead. Not critical but perhaps some remaining potential. The limit seems to be ~500 MS/s on my machine.

N.B. I had to disable some expect-statements in qa_GraphMessages (annotated 'FIXME') due to some missing runtime graph modification implementation that I believe @ivan-cukic is working on parallel in these issues/branches:

@RalphSteinhagen
Copy link
Member Author

Needed to push to fix some of my last-minute 'verschlimmbesserungen'. 😬

Did I mention I like unit-tests... saved my day.

@RalphSteinhagen RalphSteinhagen force-pushed the optionalIoBlocking branch 2 times, most recently from 4898b42 to 51e3540 Compare July 15, 2024 12:16
... in case there is no data to be processed and/or no IO activity.

* added missing processBulk(..) user return status handling
* changed ExecutionPolicy to 'enum class' (type safety) -> needed some adjustments in other unrelated unit-tests
* added msg-to-stream processing ratio
* simplified SchedulerBase and streamlined single- and multi-threaded handling
* added provisions for runtime graph modifications

Signed-off-by: rstein <[email protected]>
Signed-off-by: Ralph J. Steinhagen <[email protected]>
Signed-off-by: rstein <[email protected]>
Signed-off-by: Ralph J. Steinhagen <[email protected]>
Signed-off-by: Ralph J. Steinhagen <[email protected]>
Signed-off-by: Ralph J. Steinhagen <[email protected]>
Signed-off-by: rstein <[email protected]>
Signed-off-by: Ralph J. Steinhagen <[email protected]>
... to have the BusyLoopBlock/Graph settle into a steady state during the initially spin-up/filling of buffers

Signed-off-by: Ralph J. Steinhagen <[email protected]>
before message were at least once (often twice as often) called per processing of streaming data (which was/is a bit excessive) and now reduced to favour low-latency on sync/stream processing.

Signed-off-by: Ralph J. Steinhagen <[email protected]>
Copy link
Member

@wirew0rm wirew0rm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 👍

The failures in the CI could be verified to have been introduced by a previous commit and will be investigated and fixed separately, see #380.

@wirew0rm wirew0rm merged commit e608f23 into main Jul 18, 2024
8 of 11 checks passed
@wirew0rm wirew0rm deleted the optionalIoBlocking branch July 18, 2024 07:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: QA-Accepted/Merged (∞)
Development

Successfully merging this pull request may close these issues.

[2pt] graph-prototype: Investigate if we have unexpected runtime data processing regression/bottlenecks
3 participants