Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass arguments by const reference to avoid making copies #585

Merged
merged 2 commits into from
Apr 19, 2024
Merged

Pass arguments by const reference to avoid making copies #585

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
53ded81
Pass arguments by const reference to avoid making copies
jmcarcell Apr 15, 2024
83b55e9
Add another pass by const reference
jmcarcell Apr 16, 2024
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
4 changes: 2 additions & 2 deletions python/templates/Collection.cc.jinja2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ bool {{ collection_type }}::setReferences(const podio::ICollectionProvider* coll
return m_storage.setReferences(collectionProvider, m_isSubsetColl);
}

void {{ collection_type }}::push_back(Mutable{{ class.bare_type }} object) {
void {{ collection_type }}::push_back(const Mutable{{ class.bare_type }}& object) {
// We have to do different things here depending on whether this is a
// subset collection or not. A normal collection cannot collect objects
// that are already part of another collection, while a subset collection
Expand All @@ -156,7 +156,7 @@ void {{ collection_type }}::push_back(Mutable{{ class.bare_type }} object) {
}
}

void {{ collection_type }}::push_back({{ class.bare_type }} object) {
void {{ collection_type }}::push_back(const {{ class.bare_type }}& object) {
if (!m_isSubsetColl) {
throw std::invalid_argument("Immutable objects can only be added to subset collections");
}
Expand Down
4 changes: 2 additions & 2 deletions python/templates/Collection.h.jinja2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,9 @@ public:


/// Append object to the collection
void push_back(Mutable{{class.bare_type}} object);
void push_back(const Mutable{{class.bare_type}}& object);
/// Append an object to the (subset) collection
void push_back({{ class.bare_type }} object);
void push_back(const {{ class.bare_type }}& object);

void prepareForWrite() const final;
void prepareAfterRead() final;
Expand Down
2 changes: 1 addition & 1 deletion python/templates/macros/collections.jinja2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ podio::CollectionReadBuffers createBuffersV{{ schemaVersion }}(bool isSubset) {
{% endfor %}
}

readBuffers.createCollection = [](podio::CollectionReadBuffers buffers, bool isSubsetColl) {
readBuffers.createCollection = [](const podio::CollectionReadBuffers& buffers, bool isSubsetColl) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am not particularly liking this because it's a bit misleading, because something down the line will be stealing the data from these buffers:

podio/python/templates/CollectionData.cc.jinja2

Lines 35 to 36 in 53ded81

m_refCollections(std::move(*buffers.references)),
m_vecmem_info(std::move(*buffers.vectorMembers)) {

On the other hand this is the case already now because the copy we make at the moment will also have the same behavior. In the end all of this should be cleaned up better at some point (see #506).

Interestingly this also seems to work even though we technically assign to a different std::function:

podio/include/podio/CollectionBuffers.h

Line 51 in 53ded81

using CreateFuncT = std::function<std::unique_ptr<podio::CollectionBase>(podio::CollectionReadBuffers, bool)>;

{{ collection_type }}Data data(buffers, isSubsetColl);
return std::make_unique<{{ collection_type }}>(std::move(data), isSubsetColl);
};
Expand Down
Loading