Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions io/io/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ ROOT_LINKER_LIBRARY(RIO
src/TStreamerInfoReadBuffer.cxx
src/TStreamerInfoWriteBuffer.cxx
src/TZIPFile.cxx
src/RFile.cxx
$<TARGET_OBJECTS:RootPcmObjs>
LIBRARIES
${CMAKE_DL_LIBS}
Expand All @@ -73,6 +74,7 @@ if(uring)
endif()

ROOT_GENERATE_DICTIONARY(G__RIO
ROOT/RFile.hxx
ROOT/RRawFile.hxx
ROOT/RRawFileTFile.hxx
${rawfile_local_headers}
Expand Down
138 changes: 138 additions & 0 deletions io/io/inc/ROOT/RFile.hxx
Copy link
Member

Choose a reason for hiding this comment

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

After yesterday's discussion, is it wise to put this in io/v7? Version numbers in file names or configs will just create headaches. How about putting it in io/rfile, where it can remain also in the future?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the end I just put it in io/io to avoid CMake headaches. Since it's always built anyway it doesn't make much of a difference in my opinion.

Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
/// \file ROOT/RFile.hxx
/// \ingroup Base ROOT7
/// \author Giacomo Parolini <[email protected]>
/// \date 2025-03-19
/// \warning This is part of the ROOT 7 prototype! It will change without notice. It might trigger earthquakes. Feedback
/// is welcome!

#ifndef ROOT7_RFile
#define ROOT7_RFile

#include <ROOT/RError.hxx>

#include <memory>
#include <string_view>
#include <typeinfo>

class TFile;
class TKey;

namespace ROOT {
namespace Experimental {

class RFile;
struct RFileKeyInfo;

namespace Internal {

ROOT::RLogChannel &RFileLog();

} // namespace Internal

/**
\class ROOT::Experimental::RFile
\ingroup RFile
\brief An interface to read from, or write to, a ROOT file, as well as performing other common operations.

TODO: more in-depth explanation
*/
class RFile final {
Copy link
Member

Choose a reason for hiding this comment

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

final will prevent users from inheriting and possibly overriding. Is this the intention?

Copy link
Member

Choose a reason for hiding this comment

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

A larger docstring for the class documentation would be desirable, but it

  • can be added later
  • and could be placed in the .cxx, so you don't have to recompile everything when updating the docs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

final will prevent users from inheriting and possibly overriding. Is this the intention?

Considering we're not defining any virtual method (including the destructor) I don't see how the user could effectively inherit this class anyway. If we change our mind later we can just remove the final, but I consider it a good default.

enum PutFlags {
kPutAllowOverwrite = 0x1,
kPutOverwriteKeepCycle = 0x2,
};

std::unique_ptr<TFile> fFile;

// Outlined to avoid including TFile.h
explicit RFile(std::unique_ptr<TFile> file);

/// Gets object `path` from the file and returns an **owning** pointer to it.
/// The caller should immediately wrap it into a unique_ptr of the type described by `type`.
[[nodiscard]] void *GetUntyped(std::string_view path, const std::type_info &type) const;

/// Writes `obj` to file, without taking its ownership.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this makes it clearer?

Suggested change
/// Writes `obj` to file, without taking its ownership.
/// Writes a copy of `obj` to file, without taking its ownership.

Copy link
Member

Choose a reason for hiding this comment

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

An explanation of type and flags would be great, too.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I see now that this is private, so the request has low priority.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure about the "a copy of" part; the object is not really copied in memory, it's just serialized into the file. At least, that's the mental model that the both us and the user should have, regardless of how TFile actually implements it

void PutUntyped(std::string_view path, const std::type_info &type, const void *obj, std::uint32_t flags);

/// \see Put
template <typename T>
void PutInternal(std::string_view path, const T &obj, std::uint32_t flags)
{
PutUntyped(path, typeid(T), &obj, flags);
}

/// Given `path`, returns the TKey corresponding to the object at that path (assuming the path is fully split, i.e.
/// "a/b/c" always means "object 'c' inside directory 'b' inside directory 'a'").
/// IMPORTANT: `path` must have been validated/normalized via ValidateAndNormalizePath() (see RFile.cxx).
TKey *GetTKey(std::string_view path) const;

public:
// This is arbitrary, but it's useful to avoid pathological cases
static constexpr int kMaxPathNesting = 1000;

///// Factory methods /////

/// Opens the file for reading
static std::unique_ptr<RFile> Open(std::string_view path);

/// Opens the file for reading/writing, overwriting it if it already exists
static std::unique_ptr<RFile> Recreate(std::string_view path);

/// Opens the file for updating
static std::unique_ptr<RFile> OpenForUpdate(std::string_view path);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
static std::unique_ptr<RFile> OpenForUpdate(std::string_view path);
static std::unique_ptr<RFile> Update(std::string_view path);


///// Instance methods /////

// Outlined to avoid including TFile.h
~RFile();

/// Retrieves an object from the file.
/// `path` should be a string such that `IsValidPath(path) == true`, otherwise an exception will be thrown.
/// If the object is not there returns a null pointer.
template <typename T>
std::unique_ptr<T> Get(std::string_view path) const
{
void *obj = GetUntyped(path, typeid(T));
return std::unique_ptr<T>(static_cast<T *>(obj));
}

/// Puts an object into the file.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// Puts an object into the file.
/// Puts a copy of an object into the file.

/// The application retains ownership of the object.
/// `path` should be a string such that `IsValidPath(path) == true`, otherwise an exception will be thrown.
///
/// Throws a RException if `path` already identifies a valid object or directory.
/// Throws a RException if the file was opened in read-only mode.
template <typename T>
void Put(std::string_view path, const T &obj)
{
PutInternal(path, obj, /* flags = */ 0);
}

/// Puts an object into the file, overwriting any previously-existing object at that path.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// Puts an object into the file, overwriting any previously-existing object at that path.
/// Puts a copy of an object into the file, overwriting any previously-existing object at that path.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See comment above

/// The application retains ownership of the object.
///
/// If an object already exists at that path, it is kept as a backup cycle unless `backupPrevious` is false.
/// Note that even if `backupPrevious` is false, any existing cycle except the latest will be preserved.
///
/// Throws a RException if `path` is already the path of a directory.
/// Throws a RException if the file was opened in read-only mode.
template <typename T>
void Overwrite(std::string_view path, const T &obj, bool backupPrevious = true)
{
std::uint32_t flags = kPutAllowOverwrite;
flags |= backupPrevious * kPutOverwriteKeepCycle;
PutInternal(path, obj, flags);
}

/// Writes all objects to disk with the file structure.
/// Returns the number of bytes written.
size_t Write();
Copy link
Member

Choose a reason for hiding this comment

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

Does the number of bytes written make sense here if the other write functions return void?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Which other write functions? You mean Put?
I think it makes sense, also considering that TFile::Write() returns the information, so it makes sense to use it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
size_t Write();
size_t Flush();

or

Suggested change
size_t Write();
size_t Commit();


/// Flushes the RFile if needed and closes it, disallowing any further reading or writing.
void Close();
};

} // namespace Experimental
} // namespace ROOT

#endif
Loading
Loading