Skip to content

Simplify collection of data-layer hierarchy information#286

Merged
knoepfel merged 4 commits intoFramework-R-D:mainfrom
knoepfel:simplify-hierarchy-reporting
Feb 10, 2026
Merged

Simplify collection of data-layer hierarchy information#286
knoepfel merged 4 commits intoFramework-R-D:mainfrom
knoepfel:simplify-hierarchy-reporting

Conversation

@knoepfel
Copy link
Copy Markdown
Member

@knoepfel knoepfel commented Feb 4, 2026

This PR replaces the complicated end-of-message RAII system with a simpler approach, where a dedicated node is used to collect data-layer hierarchy information.

@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 4, 2026

Codecov Report

❌ Patch coverage is 95.65217% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
phlex/core/declared_predicate.hpp 50.00% 1 Missing ⚠️
phlex/core/declared_unfold.hpp 95.45% 0 Missing and 1 partial ⚠️

❌ Your project status has failed because the head coverage (75.62%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage.

@@            Coverage Diff             @@
##             main     #286      +/-   ##
==========================================
+ Coverage   75.28%   75.62%   +0.34%     
==========================================
  Files         126      125       -1     
  Lines        2954     2946       -8     
  Branches      519      519              
==========================================
+ Hits         2224     2228       +4     
+ Misses        505      497       -8     
+ Partials      225      221       -4     
Flag Coverage Δ
unittests 75.62% <95.65%> (+0.34%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
phlex/core/declared_fold.hpp 94.82% <100.00%> (ø)
phlex/core/declared_provider.hpp 100.00% <100.00%> (ø)
phlex/core/declared_transform.hpp 91.30% <100.00%> (-0.19%) ⬇️
phlex/core/detail/filter_impl.hpp 100.00% <ø> (ø)
phlex/core/filter.cpp 73.91% <100.00%> (-1.09%) ⬇️
phlex/core/framework_graph.cpp 90.90% <100.00%> (+0.58%) ⬆️
phlex/core/framework_graph.hpp 100.00% <ø> (ø)
phlex/core/message.hpp 94.44% <ø> (ø)
phlex/core/message_sender.cpp 56.25% <100.00%> (-11.94%) ⬇️
phlex/core/multiplexer.cpp 79.31% <100.00%> (ø)
... and 2 more

... and 3 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fe24169...a90d07c. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@knoepfel knoepfel linked an issue Feb 4, 2026 that may be closed by this pull request
@knoepfel knoepfel requested a review from Copilot February 4, 2026 20:58
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR removes the end-of-message RAII mechanism (and its end_of_message pointer propagation) and replaces hierarchy reporting with a dedicated TBB node that records data-layer information as messages enter the graph.

Changes:

  • Simplifies message/predicate_result by removing end_of_message_ptr and updates all affected nodes/tests accordingly.
  • Introduces a hierarchy_node_ in framework_graph to record data-cell indices (including those created by unfolds).
  • Deletes the end_of_message implementation and removes it from the build/install rules.

Reviewed changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
test/filter_impl.cpp Updates filter decision-map tests to the new predicate_result shape.
test/allowed_families.cpp Adjusts an execution-count assertion and relocates a synchronization FIXME.
phlex/core/multiplexer.cpp Adapts multiplexing to the new message layout (no EOM).
phlex/core/message_sender.hpp Removes hierarchy/EOM stack dependencies from the sender API.
phlex/core/message_sender.cpp Stops creating EOM objects and builds simplified messages/flushes.
phlex/core/message.hpp Removes end_of_message_ptr from message.
phlex/core/fwd.hpp Drops end_of_message forward decl and pointer alias.
phlex/core/framework_graph.hpp Removes EOM stack; adds hierarchy_node_; updates sender construction.
phlex/core/framework_graph.cpp Adds hierarchy_node_ implementation and wires it to source/unfold senders.
phlex/core/filter.cpp Removes EOM plumbing when forwarding filtered messages downstream.
phlex/core/end_of_message.hpp Deleted (no longer used).
phlex/core/end_of_message.cpp Deleted (no longer used).
phlex/core/detail/filter_impl.hpp Simplifies predicate_result by removing EOM.
phlex/core/declared_unfold.hpp Removes EOM propagation and updates message construction in unfold paths.
phlex/core/declared_transform.hpp Removes EOM handling and updates message forwarding.
phlex/core/declared_provider.hpp Removes EOM handling when producing/forwarding messages.
phlex/core/declared_predicate.hpp Stops attaching EOM to predicate results.
phlex/core/declared_fold.hpp Removes EOM forwarding when emitting folded parent stores.
phlex/core/CMakeLists.txt Removes end_of_message.* from library sources and install set.

Comment thread phlex/core/framework_graph.cpp Outdated
Comment thread test/allowed_families.cpp Outdated
@knoepfel knoepfel force-pushed the simplify-hierarchy-reporting branch 3 times, most recently from 31f17b7 to 2c2dad2 Compare February 6, 2026 20:33
@greenc-FNAL
Copy link
Copy Markdown
Contributor

Review the full CodeQL report for details.

@knoepfel knoepfel force-pushed the simplify-hierarchy-reporting branch 5 times, most recently from 0b1f4c4 to b0875de Compare February 10, 2026 16:06
@knoepfel knoepfel requested a review from greenc-FNAL February 10, 2026 17:50
Copy link
Copy Markdown
Contributor

@greenc-FNAL greenc-FNAL left a comment

Choose a reason for hiding this comment

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

I can't claim to fully understand the TBB node operations, but pending my question below, it looks sensible to me.

Comment thread phlex/core/declared_unfold.hpp Outdated
@knoepfel knoepfel force-pushed the simplify-hierarchy-reporting branch from b0875de to a90d07c Compare February 10, 2026 21:45
@knoepfel knoepfel requested a review from greenc-FNAL February 10, 2026 21:45
@knoepfel knoepfel dismissed greenc-FNAL’s stale review February 10, 2026 21:46

Replaced ++msg_counter_ increment with safer msg_counter_.fetch_add(1)

@knoepfel knoepfel merged commit 76af3af into Framework-R-D:main Feb 10, 2026
46 of 47 checks passed
@knoepfel knoepfel deleted the simplify-hierarchy-reporting branch February 10, 2026 22:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refactor caching of data products

3 participants