Using shared ptr to track iodesc use by jayeshkrishna · Pull Request #698 · E3SM-Project/scorpio

jayeshkrishna · 2026-06-06T18:23:22Z

Using a shared pointer to track I/O decomposition
usage.

This avoids the unnecessary syncs required, to flush
data, when freeing I/O decompositions

Adding test that writes and immediately frees (before file close/sync) I/O decompositions. The decomposition used for read is freed after the file is closed/deleted

We no longer use custom linked lists for I/O decomposition lists. Removing unused/stale remnants (ptr to next) of the old linked list in io_desc_t.

Moving some util functions related to I/O decomposition from pioc_support.cpp to a new file (spio_iodesc_utils.cpp) No change in source code

Converting io_desc_t from a C struct to a simple C++ struct with constructor/destructor/util functions. The io_desc_t is still not a "full fledged" C++ class (this will be done in a future PR). However this minimum change is required to use shared pointers with I/O decomps. * Adding a constructor that manages constructing the io_desc_t object (instead of custom functions that alloc memory and initialize fields) * Also adding a destructor to free decomp and some util functions to free up resources. * All members of the io_desc_t struct is still public since a lot of the code base access the members directly * The member arrays/pointers are still allocated memory explicitly using malloc/calloc (these will be changed in future PRs) * A shared pointer to the io_desc_t, stored in the global io_desc list, now manages the lifetime of I/O decomps * Modified tests accordingly * Modified asserts to account for zero sized local decomp maps

Using C++ constructors and destructors for write cache info. The members are all still public (since other parts of the code access them directly) and will be changed in a future PR

I/O decompositions are separate from files and is used by files when rearranging data (before writing the data out). Since I/O decompositions can be potentially freed before it gets used by a file, we now cache a reference to the I/O decomposition in the file. * I/O decomposition objects are ref counted using shared ptrs * Using a map to cache I/O decomps in files * In the async case cache a ref to I/O decomp in the write info object

Since we now ref count I/O decompositions correctly we no longer need the hack to sync all files before freeing decomps.

Copilot

Pull request overview

This PR changes SCORPIO’s internal I/O decomposition (iodesc) lifetime management to use std::shared_ptr, allowing decompositions to be freed by the user before file sync/close without forcing an unconditional file-level sync in PIO_freedecomp.

Changes:

Introduces an io_desc_t constructor/destructor and stores iodescs in the global list as std::shared_ptr.
Adds per-file tracking of iodesc references so pending buffered writes can safely flush even after PIO_freedecomp.
Updates async-HDF5 write op caching and expands unit/integration tests to cover “free decomp before close/sync” scenarios.

Reviewed changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
tests/general/pio_decomp_tests.F90.in	Adds a regression test that frees decomps before sync/close and verifies readback.
tests/cunit/test_spio_decomp_logger.cpp	Updates test allocation/free of `io_desc_t` to use `new/delete` with the new ctor/dtor.
tests/cunit/test_rearr.cpp	Updates rearranger unit tests to construct `io_desc_t` with the new ctor and avoid double-frees.
tests/cunit/test_pioc.cpp	Updates iodesc list tests to use `std::shared_ptr<io_desc_t>` and member cleanup helpers.
src/flib/spio_decomp.F90.in	Removes unconditional `PIOc_sync()` from `PIO_freedecomp` Fortran wrapper.
src/clib/pio_types.hpp	Converts `io_desc_t` to a C++ struct with ctor/dtor and adds `file_desc_t::io_desc_refs` for per-file iodesc refs.
src/clib/pio_internal.h	Updates iodesc list APIs to shared_ptr and declares per-file iodesc ref helpers.
src/clib/core/util/spio_iosys_utils.hpp	Adds iosystem utility header (async-service detection).
src/clib/core/util/spio_iosys_utils.cpp	Implements iosystem utility helper.
src/clib/core/util/pio_lists.cpp	Stores iodescs in the global list as `std::shared_ptr` and adds shared_ptr lookup API.
src/clib/core/util/CMakeLists.txt	Adds new util compilation unit.
src/clib/core/spio_iodesc_utils.cpp	Adds new iodesc utilities/implementation (region alloc + `io_desc_t` ctor/dtor + `PIOc_freedecomp_impl`).
src/clib/core/rearr/pio_rearrange.cpp	Tightens/adjusts input assertions for ndims/map handling.
src/clib/core/pioc.cpp	Reworks `initdecomp()` to create/store iodesc via `std::make_shared`.
src/clib/core/pioc_support.cpp	Removes legacy `malloc_iodesc` and related iodesc/region free routines; initializes `file->io_desc_refs` on open/create.
src/clib/core/pio_file.cpp	Adds per-file iodesc ref cache add/get helpers guarded by the file mutex.
src/clib/core/pio_darray.cpp	Caches iodesc refs per file on write; uses shared_ptr in async-HDF5 write enqueue.
src/clib/core/iolib/hdf5/spio_async_hdf5_utils.hpp	Updates async-HDF5 enqueue API to take `std::shared_ptr<io_desc_t>`.
src/clib/core/iolib/hdf5/spio_async_hdf5_utils.cpp	Stores iodesc as shared_ptr in write cache and cleans up allocation to `new/delete`.
src/clib/core/CMakeLists.txt	Adds new core compilation unit.

Suppressed comments (1)

src/clib/core/pio_file.cpp:518

When an iodesc ref is found, this function only erases the entry for varids[0] before returning. Any other varid->ioid entries remain, keeping the iodesc alive unnecessarily and preventing PIO_freedecomp from actually releasing the decomposition until file close.

  if(iter != siter->second.end()){
    assert(iter->second->ioid == ioid);
    /* The caller now has ownership of this iodesc */
    std::shared_ptr<io_desc_t> sp = iter->second;
    siter->second.erase(iter);

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

jayeshkrishna · 2026-07-31T16:28:22Z

+  std::shared_ptr<io_desc_t> sp_iodesc = nullptr;
+  /* First try to get cached iodesc. */
+  sp_iodesc = spio_get_iodesc_ref_from_file(file, std::vector<int>{varids, varids+nvars}, ioid);
+  if(!sp_iodesc){


Fixed and rebased branch

jayeshkrishna · 2026-07-31T16:35:16Z

+    if (!(region->start = (PIO_Offset *) calloc(ndims, sizeof(PIO_Offset))))
+    {
+        return pio_err(ios, NULL, PIO_ENOMEM, __FILE__, __LINE__,
+                        "Internal error while allocating region. Out of memory allocating %lld bytes  for start array in the I/O region", (unsigned long long) (ndims * sizeof(PIO_Offset)));
+    }


Fixed and rebased branch

jayeshkrishna · 2026-07-31T16:41:34Z

+  /* Don't add duplicates, just one ref per variable to iodesc used by file.
+   * Note: A variable could however have multiple iodescs associated with it
+   */
+  std::shared_ptr<io_desc_t> iodesc = pio_get_iodesc_sptr_from_id(ioid);
+  (*(file->io_desc_refs))[varid][ioid] = iodesc;


Fixed and rebased branch

Instead of caching the I/O decomp once per file, now caching the decomp per variable per file. This change supports multiple variables using the same I/O decomposition but written out separately (g.g. 3D and 4D vars using the same I/O decomposition)

Adding a simple util function to check if I/O system is using asynchronous I/O as a service

When writes are offloaded to async I/O procs the I/O desc is not cached in the I/O procs (since PIOc_write_darray() is not offloaded). So when async I/O (as a service) is used directly get the iodesc from ioid (instead of trying to use a cached iodesc)

Although not common a variable can use multiple I/O decomps. So add support for multiple I/O descs per variable when tracking I/O desc usage via a shared ptr. Instead of caching a single shared ptr to an I/O desc per variable we now cache multiple I/O descs (shared pointers to) per variable

Instead of handling the special case of async I/O service (where I/O desc is not cached), always fall back to querying the I/O desc from the global list when its not cached. Some unit tests call the PIOc_write_darray_multi() function directly (instead of calling PIOc_write_darray() where the I/O desc is cached), so to handle these unit tests (and async I/O service case) always fall back to looking for the I/O desc in the global list when looking for cached I/O desc fails

Code reformatting alloc_region() func No change in source code (apart from formatting)

Allocate memory for start/count arrays iff ndims > 0 (Avoid callocing 0 byte arrays)

jayeshkrishna self-assigned this Jun 6, 2026

jayeshkrishna added the enhancement label Jun 6, 2026

jayeshkrishna added 3 commits July 21, 2026 11:35

Adding test for freedecomp before close

3052c6a

Adding test that writes and immediately frees (before file close/sync) I/O decompositions. The decomposition used for read is freed after the file is closed/deleted

Rm unused linked list remnant in iodesc

1a22919

We no longer use custom linked lists for I/O decomposition lists. Removing unused/stale remnants (ptr to next) of the old linked list in io_desc_t.

Moving I/O decomp utils to spio_iodesc_utils.cpp

2f31656

Moving some util functions related to I/O decomposition from pioc_support.cpp to a new file (spio_iodesc_utils.cpp) No change in source code

jayeshkrishna force-pushed the jayeshkrishna/track_iodesc_via_sptr branch 2 times, most recently from fb0d84b to 32ff469 Compare July 28, 2026 18:37

jayeshkrishna added 4 commits July 28, 2026 13:45

Using C++ const/dest for wcache info

28b19b6

Using C++ constructors and destructors for write cache info. The members are all still public (since other parts of the code access them directly) and will be changed in a future PR

Rm sync hack when freeing decomp

2c59543

Since we now ref count I/O decompositions correctly we no longer need the hack to sync all files before freeing decomps.

jayeshkrishna force-pushed the jayeshkrishna/track_iodesc_via_sptr branch from 32ff469 to 2c59543 Compare July 28, 2026 18:45

jayeshkrishna requested a review from Copilot July 31, 2026 15:07

Copilot started reviewing on behalf of jayeshkrishna July 31, 2026 15:07 View session

Copilot AI reviewed Jul 31, 2026

View reviewed changes

jayeshkrishna added 3 commits July 31, 2026 11:00

Cache iodesc per var per file

7aa090f

Instead of caching the I/O decomp once per file, now caching the decomp per variable per file. This change supports multiple variables using the same I/O decomposition but written out separately (g.g. 3D and 4D vars using the same I/O decomposition)

Adding a util func to check async mode

3f5e89f

Adding a simple util function to check if I/O system is using asynchronous I/O as a service

jayeshkrishna force-pushed the jayeshkrishna/track_iodesc_via_sptr branch from 673f8b9 to 3236839 Compare July 31, 2026 16:28

jayeshkrishna added 4 commits July 31, 2026 11:40

Code reformat alloc_region()

90f39c3

Code reformatting alloc_region() func No change in source code (apart from formatting)

In alloc_region() alloc mem iff ndims > 0

94e9f66

Allocate memory for start/count arrays iff ndims > 0 (Avoid callocing 0 byte arrays)

jayeshkrishna force-pushed the jayeshkrishna/track_iodesc_via_sptr branch from 0aa454b to 94e9f66 Compare July 31, 2026 16:41

jayeshkrishna requested a review from amametjanov July 31, 2026 19:34

jayeshkrishna marked this pull request as ready for review July 31, 2026 19:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Using shared ptr to track iodesc use - #698

Using shared ptr to track iodesc use#698
jayeshkrishna wants to merge 14 commits into
masterfrom
jayeshkrishna/track_iodesc_via_sptr

jayeshkrishna commented Jun 6, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

jayeshkrishna Jul 31, 2026

Uh oh!

jayeshkrishna Jul 31, 2026

Uh oh!

jayeshkrishna Jul 31, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

jayeshkrishna commented Jun 6, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

jayeshkrishna Jul 31, 2026

Choose a reason for hiding this comment

Uh oh!

jayeshkrishna Jul 31, 2026

Choose a reason for hiding this comment

Uh oh!

jayeshkrishna Jul 31, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants