Form read write split by aolivier23 · Pull Request #442 · Framework-R-D/phlex

aolivier23 · 2026-03-23T15:41:27Z

This PR splits form_interface into two components: form_reader_interface and form_writer_interface. This split saves FORM time by avoiding lock contention between read and write processes and prepares for RNTuple support that does not have members for an RNTupleWriter when only reading from a container.

All of the layers that power the former form_interface are split in this way. A handful of functions shared between reading and writing have been made free functions in form::experimental::detail.

form_interface is now split between form_reader_interface and form_writer_interface for example.

been replaced by PersistenceReader and PersistenceWriter.

in persistence directory.

Removed a few files that were restored in the merge with main accidentally.

aolivier23 · 2026-03-23T15:43:13Z

@phlexbot format

github-actions · 2026-03-23T15:44:34Z

Format Fixes Applied

✅ clang-format fixes pushed (commit d586ade)
✅ cmake-format fixes pushed (commit 0e84f5f)
✅ ruff fixes pushed (commit ee7bae1)
✅ YAML formatter fixes pushed (commit 23e120b)

⚠️ Note: Some issues may require manual review.

greenc-FNAL · 2026-03-23T15:46:03Z

Review the full CodeQL report for details.

codecov · 2026-03-23T15:53:43Z

Codecov Report

❌ Patch coverage is 83.21995% with 74 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
form/root_storage/root_tbranch_read_container.cpp	76.78%	8 Missing and 5 partials ⚠️
form/root_storage/root_tbranch_write_container.cpp	75.47%	9 Missing and 4 partials ⚠️
form/root_storage/root_ttree_write_container.cpp	50.00%	6 Missing and 6 partials ⚠️
form/storage/storage_reader.cpp	82.60%	4 Missing and 4 partials ⚠️
form/storage/storage_writer.cpp	84.78%	4 Missing and 3 partials ⚠️
form/form/form_writer.cpp	77.77%	3 Missing and 3 partials ⚠️
form/util/factories.hpp	77.77%	0 Missing and 4 partials ⚠️
form/persistence/persistence_reader.cpp	84.21%	2 Missing and 1 partial ⚠️
form/root_storage/root_ttree_read_container.cpp	70.00%	2 Missing and 1 partial ⚠️
form/form/form_reader.cpp	83.33%	1 Missing and 1 partial ⚠️
... and 2 more

@@            Coverage Diff             @@
##             main     #442      +/-   ##
==========================================
+ Coverage   85.00%   85.30%   +0.29%     
==========================================
  Files         127      145      +18     
  Lines        3341     3429      +88     
  Branches      574      584      +10     
==========================================
+ Hits         2840     2925      +85     
+ Misses        306      304       -2     
- Partials      195      200       +5

Flag	Coverage Δ
unittests	`85.30% <83.21%> (+0.29%)`	⬆️

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

Files with missing lines	Coverage Δ
form/form/config.cpp	`100.00% <ø> (ø)`
form/form/config.hpp	`100.00% <100.00%> (ø)`
form/form/form_reader.hpp	`100.00% <100.00%> (ø)`
form/form/form_writer.hpp	`100.00% <100.00%> (ø)`
form/form_module.cpp	`89.36% <100.00%> (+0.23%)`	⬆️
form/persistence/ipersistence_reader.hpp	`100.00% <100.00%> (ø)`
form/persistence/ipersistence_writer.hpp	`100.00% <100.00%> (ø)`
form/persistence/persistence_reader.hpp	`100.00% <100.00%> (ø)`
form/persistence/persistence_writer.cpp	`100.00% <100.00%> (ø)`
form/persistence/persistence_writer.hpp	`100.00% <100.00%> (ø)`
... and 28 more

... and 7 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 3bda620...39bb52e. Read the comment docs.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

uses aren't available yet.

…/phlex into form_read_write_split

code coverage test.

reading.

aolivier23 · 2026-03-23T23:07:48Z

@phlexbot format

github-actions · 2026-03-23T23:08:42Z

Format Fixes Applied

✅ clang-format fixes pushed (commit c47a8c6)

⚠️ Note: Some issues may require manual review.

Storage[Reader|Writer] member variables.

…/phlex into form_read_write_split

wwuoneway

Thanks @aolivier23 and @gemmeren for the constructive comments and productive conversation. I agree with the proposed path: let's proceed by renaming output_item_config (to something more neutral like Itemconfig or item_config) to resolve the immediate naming conflict/confusion, and then complete/merge this PR. We can address the remaining know issues regarding configuration/getToken() and the technology versioning in a follow-up PR to keep this structural split manageable.

friendly to reader components. We eventually plan to remove the need for ItemConfig from readers entirely, but that will be a future PR.

names.

aolivier23 · 2026-03-24T21:06:28Z

Realized I didn't rename the function configureOutputItems. Will fix in just a few minutes.

"output".

aolivier23 · 2026-03-24T21:15:05Z

@phlexbot format

github-actions · 2026-03-24T21:15:59Z

Format Fixes Applied

✅ clang-format fixes pushed (commit 39bb52e)

⚠️ Note: Some issues may require manual review.

weren't using it after the read/write split.

…/phlex into form_read_write_split

aolivier23 · 2026-03-25T21:19:59Z

@phlexbot format

github-actions · 2026-03-25T21:20:50Z

Format Fixes Applied

✅ clang-format fixes pushed (commit 888c71e)

⚠️ Note: Some issues may require manual review.

wwuoneway

thanks for the updates. The changes look fine, and given the scope and complexity already included in this PR, I'm comfortable moving forward. Regarding the CI failures, the clang-tidy is confirmed as unrelated to these specific changes. For code coverage, the cause is unclear, but I agree it shouldn't block this merge.

gemmeren

LGTM, the only comment is that if we consider the use of output_config by reader to be temporary, then we don't really need to rename it. Up to you.

aolivier23 added 9 commits February 27, 2026 13:33

Split all FORM components into reading and writing components.

2ffcd6b

form_interface is now split between form_reader_interface and form_writer_interface for example.

Removing features from old prototype branch.

0be8f74

Removed old unified Persistence class that is no longer needed. It has

75fdbc5

been replaced by PersistenceReader and PersistenceWriter.

Removed old unified Storage class that is no longer used.

471b535

Renamed storage_util.hpp to storage_utils.hpp to match naming convention

b7ddde1

in persistence directory.

Merge branch 'main' into form_read_write_split

c0071a7

Got latest FORM Storage tests working with the read/write split.

e464bda

Removed a few files that were restored in the merge with main accidentally.

Merge branch 'main' into form_read_write_split

3e7360e

Merge branch 'main' into form_read_write_split

aac5c6a

aolivier23 requested review from gemmeren and wwuoneway March 23, 2026 15:41

github-actions Bot added 4 commits March 23, 2026 15:43

Apply YAML formatter fixes

23e120b

Apply cmake-format fixes

0e84f5f

Apply clang-format fixes

d586ade

Apply ruff fixes

ee7bae1

aolivier23 added 4 commits March 23, 2026 11:00

Removed accidental RNTuple inclusion from factories. The classes it

a28c500

uses aren't available yet.

Merge branch 'main' into form_read_write_split

0c07e0a

Merge branch 'form_read_write_split' of https://github.com/aolivier23…

4346c13

…/phlex into form_read_write_split

Removed accidental mention of RNTuple components that was setting off

7c9742a

code coverage test.

gemmeren requested changes Mar 23, 2026

View reviewed changes

aolivier23 added 2 commits March 23, 2026 17:00

Removed attribute for TBranch read container that is not relevant to

82aa92f

reading.

Removed Write command in TTree read container that's no longer needed.

0153e49

Apply clang-format fixes

c47a8c6

Gave more specific names to Persistence[Reader|Writer] and

f3c5f47

Storage[Reader|Writer] member variables.

Merge branch 'form_read_write_split' of https://github.com/aolivier23…

fa895ed

…/phlex into form_read_write_split

wwuoneway previously approved these changes Mar 24, 2026

View reviewed changes

Comment thread form/form/form_reader.cpp

aolivier23 added 2 commits March 24, 2026 15:54

Changed name of class output_item_config to ItemConfig to be more

aff549f

friendly to reader components. We eventually plan to remove the need for ItemConfig from readers entirely, but that will be a future PR.

Renamed ItemConfig variables so they no longer have "output" in their

8b806c4

names.

aolivier23 dismissed wwuoneway’s stale review via 8b806c4 March 24, 2026 21:04

Removed more instances of FORM configuration variable names with

cf4f9c5

"output".

Apply clang-format fixes

39bb52e

aolivier23 added 2 commits March 25, 2026 16:15

Removed FORM's Storage_Read_Association and related machinery. We

9581913

weren't using it after the read/write split.

Merge branch 'form_read_write_split' of https://github.com/aolivier23…

871ecba

…/phlex into form_read_write_split

Apply clang-format fixes

888c71e

aolivier23 mentioned this pull request Mar 26, 2026

FORM Shall Locate Data Products without Using Configuration #461

Open

aolivier23 requested review from gemmeren and wwuoneway March 26, 2026 15:55

wwuoneway approved these changes Mar 27, 2026

View reviewed changes

aolivier23 marked this pull request as ready for review March 27, 2026 15:08

gemmeren approved these changes Mar 27, 2026

View reviewed changes

wwuoneway merged commit 863ec30 into Framework-R-D:main Mar 27, 2026
39 of 42 checks passed

Conversation

aolivier23 commented Mar 23, 2026

Uh oh!

aolivier23 commented Mar 23, 2026

Uh oh!

github-actions Bot commented Mar 23, 2026

Format Fixes Applied

Uh oh!

greenc-FNAL commented Mar 23, 2026

Uh oh!

codecov Bot commented Mar 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aolivier23 commented Mar 23, 2026

Uh oh!

github-actions Bot commented Mar 23, 2026

Format Fixes Applied

Uh oh!

wwuoneway left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

aolivier23 commented Mar 24, 2026

Uh oh!

aolivier23 commented Mar 24, 2026

Uh oh!

github-actions Bot commented Mar 24, 2026

Format Fixes Applied

Uh oh!

aolivier23 commented Mar 25, 2026

Uh oh!

github-actions Bot commented Mar 25, 2026

Format Fixes Applied

Uh oh!

wwuoneway left a comment

Choose a reason for hiding this comment

Uh oh!

gemmeren left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov Bot commented Mar 23, 2026 •

edited

Loading