Refactor backend library loading and rename Arrow suffix to PodioArrow by arnavdham · Pull Request #999 · AIDASoft/podio

arnavdham · 2026-08-04T13:16:27Z

Make Arrow mapper registration follow the same runtime-loading model as SIO, while refactoring the underlying library loading logic for both backends into a generalized, thread-safe loader.

Consolidates dynamic library loading into a single podio::utilities::BackendLibraryLoader used by both SIO and Arrow.
Ensures the library loader initialization is strictly thread-safe using C++11 magic statics, fixing potential data races.
Renames the generated Arrow plugin suffix from Arrow to PodioArrow (e.g. libedm4hepPodioArrow.so) to avoid potential clashes with libarrow.so during dynamic LD_LIBRARY_PATH scanning.

BEGINRELEASENOTES

Refactoring: Generalized dynamic library loading for I/O backends into a shared, thread-safe BackendLibraryLoader.
Change: Changed the library suffix generated by PODIO_ADD_ARROW from Arrow to PodioArrow to prevent dynamic loading collisions. Downstream projects will need to update their CMake aliases accordingly.
Fix: Arrow mapper libraries are now loaded dynamically at registry lookup time instead of relying on linker side-effects.

ENDRELEASENOTES

veprbl · 2026-08-06T16:29:10Z

+ * Registration is expected to happen during shared library loading/startup,
+ * before worker threads query the registry. After registration the registry is


A bit misleading, the loading happens lazily on getConverter/getReader calls, so users must not use those in multi-threaded context.

But this initialization is thread safe. Once lazy loading is complete MT is completely fine.

Right, so it does matter when lazy loading happens. Races can occur if several threads start up and simultaneously try to initialize reader.

We can discuss this in our meet but I think the arrowConverterLibraryLoader will only be initialsed once and as it is a local static variable it is thread safe. https://stackoverflow.com/questions/8102125/is-local-static-variable-initialization-thread-safe-in-c11

This makes sense now. We agreed, the text can be improved to specify which library this sentence is talking about (libpodioArrow or libedm4hepArrow etc)

Better comment added.

I think this one is not correct, libpodioArrow.so is not a data model, so registry does not get updated when that one is loaded (it may contain the main symbol for the registry).

So I just remove libpodioArrow.so as example?

This looks good, thank you.

veprbl · 2026-08-06T16:32:49Z

+
+    for (auto& lib : fs::directory_iterator(dir)) {
+      const auto filename = lib.path().filename().string();
+      if (filename.find("Arrow") != std::string::npos && filename.find("libarrow") == std::string::npos) {


Why can it be either one? libarrow must come from arrow-cpp, which libpodioArrow is meant to link against, so it gets loaded as a dependency.

Maybe check for ArrowMapper?

I believe, we agreed not to name the library Mapper, but simply Arrow.

Only checking for Arrow now.

jmcarcell · 2026-08-10T10:54:48Z

+
+    for (auto& lib : fs::directory_iterator(dir)) {
+      const auto filename = lib.path().filename().string();
+      if (filename.find("Arrow") != std::string::npos) {


I think this is ok because of the capital A; in a view the directories in LD_LIBRARY_PATH may contain lots of libraries, and in particular libarrow.so, which doesn't match. But if there were multiple podio datamodels that means all the arrow libraries are opened. This is just a comment, since I had to think about this case and check which names are used in LCG stacks (there doesn't seem to be anything else with Arrow in their name in the top level folders).

You could also give this a slightly less generic suffix that is less likely to clash. That might require to change parts of the code gen in some cmake macro I would assume (without having checked).

For this to work, I think we will have to make changes downstream(edm4hep) as well so I would suggest keeping it as Arrow for simplicity.

A more impactful improvement would be to load only a relevant library for the model(s) used in a file, which would likely also resolve this.

I don't think getting the library would be easy. Also we don't have files right now as everything is in memory

For this to work, I think we will have to make changes downstream(edm4hep) as well so I would suggest keeping it as Arrow for simplicity.

I don't think there has been a podio release yet with the functionality, so the most we would break at this point is some nightly builds for which we don't make too many guarantees. So if we want to make this change, now is essentially the time.

A more impactful improvement would be to load only a relevant library for the model(s) used in a file, which would likely also resolve this.

There is generally room for improvement here, see also: #403 I don't think we need to worry too much about performance here in general. There have been some interesting interactions with cvmfs and root dictionary loading, but realistically people will have only a (very) small number of datamodels to consider and even the fallback of going through LD_LIBRARY_PATH will only do a scan for matching libraries once.

I don't think there has been a podio release yet with the functionality, so the most we would break at this point is some nightly builds for which we don't make too many guarantees. So if we want to make this change, now is essentially the time.

Would you like to suggest a name? We had ArrowMapper which I did not like at the time.

There is generally room for improvement here, see also: #403 I don't think we need to worry too much about performance here in general.

Right, my point was that loading an irrelevant Arrow library is on the same scale of a problem.

I don't think getting the library would be easy. Also we don't have files right now as everything is in memory

A library name can go to Table metadata.

Would you like to suggest a name? We had ArrowMapper which I did not like at the time.

I don't have a strong opinion on this one. One could even do PodioArrow (or any variation of that) to be very specific. Given that all the builds and environment building are pretty well automated, I don't think too many people will look at the generated library names in any case.

jmcarcell · 2026-08-10T13:46:40Z

Any more comments @veprbl? Looks good to me as it is

tmadlener

At least the library loading part looks like an almost copy of what we do for SIO (i.e. SIOBlockLibraryLoader is essentially ArrowConverterLibraryLoader with some string replacements). Can those two be generalized into one configurable library loader that works for both? Configuration would probably take the env variable name, a library pattern and probably some designator for the log messages.

tmadlener · 2026-08-10T14:38:41Z

+
+    for (auto& lib : fs::directory_iterator(dir)) {
+      const auto filename = lib.path().filename().string();
+      if (filename.find("Arrow") != std::string::npos) {


You could also give this a slightly less generic suffix that is less likely to clash. That might require to change parts of the code gen in some cmake macro I would assume (without having checked).

arnavdham · 2026-08-10T15:53:03Z

At least the library loading part looks like an almost copy of what we do for SIO (i.e. SIOBlockLibraryLoader is essentially ArrowConverterLibraryLoader with some string replacements). Can those two be generalized into one configurable library loader that works for both? Configuration would probably take the env variable name, a library pattern and probably some designator for the log messages.

I could do this.

tmadlener

Thanks for pulling out the generic bits, looks good to me.

I am wondering whether we want the fallback to LD_LIBRARY_PATH for Arrow as well. For SIO it's essentially mainly there because we need it for some backwards compatibility, which is not really a concern yet for Arrow.

arnavdham · 2026-08-11T12:20:29Z

Thanks for pulling out the generic bits, looks good to me.

I am wondering whether we want the fallback to LD_LIBRARY_PATH for Arrow as well. For SIO it's essentially mainly there because we need it for some backwards compatibility, which is not really a concern yet for Arrow.

So no scanning if podio_arrow_path is not set? Maybe bool allowFallbackToLDLibraryPath can be added to the constructor of BackendLibraryLoader set to true for SIO and false for arrow? @veprbl @tmadlener

tmadlener · 2026-08-11T15:21:00Z

So no scanning if podio_arrow_path is not set?

Yes, exactly. I have no strong opinion here.

Maybe bool allowFallbackToLDLibraryPath can be added to the constructor of BackendLibraryLoader set to true for SIO and false for arrow?

If we decide to not have the fallback for arrow that would be the technical way of doing it probably.

veprbl · 2026-08-11T16:22:11Z

I'm not in favor of removing LD_LIBRARY_PATH. If anything, non-standard PODIO_ARROW_PATH should be retired.

arnavdham · 2026-08-11T17:02:30Z

I'm not in favor of removing LD_LIBRARY_PATH. If anything, non-standard PODIO_ARROW_PATH should be retired.

So I keep the current implementation and just change Arrow to PodioArrow to avoid clashes and similar change in edm4hep and its should be fine? @veprbl @tmadlener

veprbl · 2026-08-11T17:13:23Z

I'm not in favor of removing LD_LIBRARY_PATH. If anything, non-standard PODIO_ARROW_PATH should be retired.

So I keep the current implementation and just change Arrow to PodioArrow to avoid clashes and similar change in edm4hep and its should be fine? @veprbl @tmadlener

Let's do that, please.

tmadlener · 2026-08-11T17:28:00Z

So I keep the current implementation and just change Arrow to PodioArrow to avoid clashes and similar change in edm4hep and its should be fine?

I don't think any changes in edm4hep would be necessary? Shouldn't a corresponding change in the PODIO_ADD_ARROW macro already propagate through all the machinery transparently?

arnavdham · 2026-08-11T17:31:17Z

So I keep the current implementation and just change Arrow to PodioArrow to avoid clashes and similar change in edm4hep and its should be fine?

I don't think any changes in edm4hep would be necessary? Shouldn't a corresponding change in the PODIO_ADD_ARROW macro already propagate through all the machinery transparently?

https://github.com/key4hep/EDM4hep/pull/502/changes the changes made here would have to be renamed from edm4hepArrow to edm4hepPodioArrow

arnavdham · 2026-08-11T17:42:10Z

Have renamed to PodioArrow

tmadlener · 2026-08-11T17:43:08Z

Ah yes, missed the fact that we use the target names directly there. In that case this would need a change in EDM4hep as well.

arnavdham · 2026-08-11T17:46:11Z

key4hep/EDM4hep#509 Change in edm4hep

tmadlener · 2026-08-12T11:46:48Z

@@ -28,11 +30,16 @@ void ArrowConverterRegistry::registerReader(const std::string& typeName, BufferR
 }

 ArrowConverterRegistry::BufferReaderFunc ArrowConverterRegistry::getReader(const std::string& typeName) const {
+  loadArrowLibraries();
  auto it = m_readerRegistry.find(typeName);
  if (it != m_readerRegistry.end()) {
    return it->second;
  }
  return nullptr;
 }


Sorry for being rather slow here, but IIUC, this might happen on multiple threads concurrently here. The getReader (potentially also the getConverter, but there I haven't fully understood the logic yet, I think) is called in a section which podio assumes is safe to be called from multiple threads. In SIO we load the libraries during the construction of the reader. This can't be placed into the ArrowConverterRegistry constructor because we will actually try to call into that from the loaded libraries, I think. But maybe there is some place where we have slightly less potential problems with trying to load from multiple threads.

That was the question we had too. In the end, it seems like Arnav's implementation relies on a guarantee that static local variable initialization is thread safe in C++.

Ah yes, good point. I missed the static assignment in the loadXYZLibraries. In that case I think threading should pose no issues and, I think this also gives us a guarantee of happening exactly once.

So ig this can be merged now if there are no more comments?

arnavdham added 2 commits August 4, 2026 18:44

dynamic loading

cda9831

In type registry also

f3b8fdf

arnavdham mentioned this pull request Aug 4, 2026

Implement dynamic loading for Arrow mapper libraries on registry lookup #1000

Open

comments for clarification

19886bd

veprbl reviewed Aug 6, 2026

View reviewed changes

jmcarcell reviewed Aug 7, 2026

View reviewed changes

Comment thread src/ArrowConverterRegistry.cc Outdated

jmcarcell reviewed Aug 7, 2026

View reviewed changes

Comment thread src/ArrowConverterRegistry.cc Outdated

jmcarcell reviewed Aug 7, 2026

View reviewed changes

Comment thread src/ArrowTypeRegistry.cc Outdated

Comments resolution.

b729d85

arnavdham requested a review from jmcarcell August 8, 2026 06:25

jmcarcell reviewed Aug 10, 2026

View reviewed changes

tmadlener reviewed Aug 10, 2026

View reviewed changes

removing libpodioArrow.so

8d4fba2

generic library loader

d6d263b

arnavdham requested a review from tmadlener August 10, 2026 16:45

tmadlener reviewed Aug 10, 2026

View reviewed changes

Comment thread include/podio/utilities/LibraryLoader.h Outdated

Comment thread src/ArrowConverterRegistry.cc Outdated

Comment thread src/SIOBlock.cc Outdated

resolving comments

b2dbb4a

This was referenced Aug 11, 2026

[DRAFT POC] Add Arrow IPC stream writer for EDM4hep output AIDASoft/DD4hep#1658

Draft

[DRAFT POC] Add Arrow IPC stream reader for EDM4hep input eic/EICrecon#2857

Draft

Arrow->PodioAroow

73757f0

arnavdham mentioned this pull request Aug 11, 2026

Update CMake configuration to use new PodioArrow library suffix key4hep/EDM4hep#509

Open

arnavdham changed the title ~~Load Arrow mapper libraries on registry lookup~~ Refactor backend library loading and rename Arrow suffix to PodioArrow Aug 11, 2026

tmadlener reviewed Aug 12, 2026

View reviewed changes

		* Registration is expected to happen during shared library loading/startup,
		* before worker threads query the registry. After registration the registry is

Uh oh!

Conversation

arnavdham commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmcarcell commented Aug 10, 2026

Uh oh!

tmadlener left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arnavdham commented Aug 10, 2026

Uh oh!

tmadlener left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

arnavdham commented Aug 11, 2026

Uh oh!

tmadlener commented Aug 11, 2026

Uh oh!

veprbl commented Aug 11, 2026

Uh oh!

arnavdham commented Aug 11, 2026

Uh oh!

veprbl commented Aug 11, 2026

Uh oh!

tmadlener commented Aug 11, 2026

Uh oh!

arnavdham commented Aug 11, 2026

arnavdham commented Aug 4, 2026 •

edited

Loading