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.

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

Pseudo batch-less operations #178

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

t-erxleben
Copy link
Contributor

It is now possible to pass J_SINGLE_OP to operations instead of a batch. The operation will then be directly executed using default semantics.

* Default semantic settings will be used for execution.
*
*/
#define J_SINGLE_OP NULL
Copy link
Member

Choose a reason for hiding this comment

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

I think the name should include batch, maybe J_BATCH_SINGLE?

Comment on lines 276 to 279
if (batch == J_SINGLE_OP)
{
return j_semantics_new(J_SEMANTICS_TEMPLATE_DEFAULT);
}
Copy link
Member

Choose a reason for hiding this comment

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

We could also allocate a semantics singleton so callers do not have to free the semantics. I'm not sure whether it's worth the effort, though.

@@ -102,6 +112,8 @@ G_DEFINE_AUTOPTR_CLEANUP_FUNC(JBatch, j_batch_unref)
/**
* Returns a batch's semantics.
*
* The returned object must be freed using j_semantic_unref.
Copy link
Member

Choose a reason for hiding this comment

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

Does this affect any existing code? It could lead to memory leaks.

lib/item/jitem.c Outdated
Comment on lines 249 to 250
ret &= j_kv_delete(item->kv, batch);
ret &= j_distributed_object_delete(item->object, batch);
Copy link
Member

Choose a reason for hiding this comment

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

Is this correct? & is not necessarily the same as &&.

@michaelkuhn michaelkuhn self-assigned this Mar 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants