root-project / root

The official repository for ROOT: analyzing, storing and visualizing big data, scientifically
https://root.cern
Other
2.7k stars 1.28k forks source link

Spurious(?) warnings persisting interpreted classes #9371

Open bendavid opened 2 years ago

bendavid commented 2 years ago

In some cases when trying to persist classes from ACLIC or the interpreter, there are warnings about missing dictionaries, which apparently don't prevent correctly persisting the class.

Real use case is persisting boost histogram templates instantiated with PyROOT, but a standalone example is included below.

#include "TFile.h"
#include <atomic>
#include <iostream>

class Test {
public:
  std::atomic<double> &data() { return data_; }
  const std::atomic<double> &data() const { return data_; }

private:
  std::atomic<double> data_;
};

class Test2 : public std::atomic<double> {
};

int testio() {

  Test t;
  t.data() = 5.;

  Test2 t2;
  t2.store(5.);

  TFile *fout = TFile::Open("test.root", "RECREATE");

  fout->WriteObject(&t, "t");
  fout->WriteObject(&t2, "t2");

  fout->Close();

  TFile *fin = TFile::Open("test.root");

  auto &tread = *fin->Get<Test>("t");
  auto &tread2 = *fin->Get<Test2>("t2");

  std::cout << tread.data() << std::endl;
  std::cout << tread2 << std::endl;

  return 0;
}

The output is:

[bendavid@docker@lxplus8s10 boostiotest]$ root -l testio.cpp+
root [0] 
Processing testio.cpp+...
Info in <TUnixSystem::ACLiC>: creating shared library /home/b/bendavid/boostiotest/./testio_cpp.so
Warning in <TStreamerInfo::Build>: Test: atomic<double> has no streamer or dictionary, data member "data_" will not be saved
Warning in <TStreamerInfo::Build>: Test2: base class atomic<double> has no streamer or dictionary it will not be saved
5
5
(int) 0

So apparently both classes (and the std::atomic data member and/or base class) are persisted just fine.

pcanal commented 2 years ago

Sorta. If (and it look like it here) it succeeded in writing it, it did not in a likely platform dependent manners (i.e. recording the complete set of implementation dependent class hierarchy in the currently used std library).

We had a similar problem for std::unique_ptr which we solved by the I/O "ignoring", per se, the implementation and treating it memory content almost the same as a raw pointer.

We could to do something similar for std::atomic but that would circumvent the thread-safety parts; so the real solution needs more thoughts :(

bendavid commented 2 years ago

Thanks, One thing which confused me here is that there is no warning if I try to persist a std::atomic directly (ie not as a base class or a data member), but I guess the same potential issue is present regardless.

pcanal commented 2 years ago

humm ... that might be a fluke (that there is no warning in the direct case), On MacOS, I do:

root [0] f = TFile::Open("empty.root", "RECREATE");
root [1] std::atomic<double> ad
(std::atomic<double> &) @0x101594eb8
root [2] f->WriteObject(&ad,"ad")
Warning in <TStreamerInfo::Build>: __atomic_base<double,false>: __cxx_atomic_impl<double,__cxx_atomic_base_impl<double> > has no streamer or dictionary, data member "__a_" will not be saved
Warning in <TStreamerInfo::Build>: atomic<double>: base class __atomic_base<double,false> has no streamer or dictionary it will not be saved
Error in <TStreamerInfo::Build>: __cxx_atomic_base_impl<double>, unknown type: _Atomic(double) __a_value

Warning in <TStreamerInfo::Build>: __cxx_atomic_impl<double,__cxx_atomic_base_impl<double> >: base class __cxx_atomic_base_impl<double> has no streamer or dictionary it will not be saved
(int) 97

(and reading it back does not seem to work either).

bendavid commented 2 years ago

A similar warning is also provoked in the simpler case below, so it's not only related to STL stuff:

#include "TFile.h"

class TestBase {};

class Test : public TestBase {};

void testio() {

  Test t;

  TFile *fout = TFile::Open("test.root", "RECREATE");

  fout->WriteObjectAny(&t, TClass::GetClass<Test>(), "t");
}
root -l testio.cpp 
root [0] 
Processing testio.cpp...
Warning in <TStreamerInfo::Build>: Test: base class TestBase has no streamer or dictionary it will not be saved
root [1] .q
bendavid commented 2 years ago

And for this I think the problem is "just" the checks on TClass::IsLoaded() at

https://github.com/root-project/root/blob/702e1ee22ce472508d0ab2de7f51b85e6613b55d/io/io/src/TStreamerInfo.cxx#L384 https://github.com/root-project/root/blob/702e1ee22ce472508d0ab2de7f51b85e6613b55d/io/io/src/TStreamerInfo.cxx#L580 https://github.com/root-project/root/blob/702e1ee22ce472508d0ab2de7f51b85e6613b55d/io/io/src/TStreamerInfo.cxx#L592

Since the state of the TClass object is kInterpreted in this case.

Not sure what the best solution here is. On the one hand being able to persist interpreted classes is extremely useful (especially for the case of templates which are only instantiated from PyROOT), but it's also clear that one can shoot themselves in the foot here if the interpreted class changes in a way that breaks the IO.

pcanal commented 2 years ago

The support for I/O of interpreted classes is not complete (it is missing at least support for STL collection). Even-though that some classes could be stored, we choose (for now?) to leave the message for all of them (rather than trying to test for the subset that 'works' and have a mixed up message)

bendavid commented 2 years ago

Ok noted, thanks. The current situation is pretty confusing though because

1) There is no warning when trying to directly persist an interpreted class, only when it is a base class or data member 2) Trying to persist a data member which is a pointer to an interpreted class is an Error rather than a warning 3) The warning itself is not really accurate because the data member or base class may actually be saved.

In principle the level of warning/error and actual behaviour should be the same in all those cases I guess.

ie in the case below

#include "TFile.h"

class TestBase {};

class TestMember {};

class Test : public TestBase {};

class Test2 {
  TestMember data_;
};

class Test3 {
  TestMember *data_ = nullptr;
};

void testio() {

  Test tb;
  Test t;
  Test2 t2;
  Test3 t3;

  TFile *fout = TFile::Open("test.root", "RECREATE");

  fout->WriteObjectAny(&tb, TClass::GetClass<TestBase>(), "tb");
  fout->WriteObjectAny(&t, TClass::GetClass<Test>(), "t");
  fout->WriteObjectAny(&t2, TClass::GetClass<Test2>(), "t2");
  fout->WriteObjectAny(&t3, TClass::GetClass<Test3>(), "t3");

}
root -l testio.cpp 
root [0] 
Processing testio.cpp...
Warning in <TStreamerInfo::Build>: Test: base class TestBase has no streamer or dictionary it will not be saved
Warning in <TStreamerInfo::Build>: Test2: TestMember has no streamer or dictionary, data member "data_" will not be saved
Error in <TStreamerInfo::Build>: Test3: TestMember* has no streamer or dictionary, data member data_ will not be saved
hahnjo commented 1 month ago

FWIW I hit this in https://github.com/root-project/root/pull/16172 where I was confused why only the case of base classes was reported...

guitargeek commented 1 month ago

Is this connected to #16202?

hahnjo commented 1 month ago

Is this connected to #16202?

No, there is no transiency involved here, only inheritance.