Skip to content

[NFC] Factor out initialization of em_queued_call arguments #15686

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

Merged
merged 2 commits into from
Dec 2, 2021

Conversation

tlively
Copy link
Member

@tlively tlively commented Dec 1, 2021

No description provided.

This is the first step toward decoupling the logic for proxying work to threads
from the logic for creation and dispatch of `em_queued_call` objects, which
couple a dynamically typed function pointer with its arguments. Decoupling these
pieces will allow for more general proxying that is not required to use the
`em_queued_call` mechanism and will simplify the code.
@kripken kripken requested a review from juj December 1, 2021 19:27
Copy link
Collaborator

@sbc100 sbc100 left a comment

Choose a reason for hiding this comment

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

Nice!

static void init_em_queued_call_args(em_queued_call* q,
EM_FUNC_SIGNATURE sig,
va_list args) {
EM_FUNC_SIGNATURE argumentsType = sig & EM_FUNC_SIG_ARGUMENTS_TYPE_MASK;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you can do q->functionEnum = sig; here too.. in fact maybe just pass func_ptr too and call this init_em_queued_call

Copy link
Member Author

Choose a reason for hiding this comment

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

In an upcoming PR I add a function similar to what you suggest that allocates and initializes the full em_queued_call. The reason to bundle the initialization with allocation is that it is much easier to manually initialize a stack-allocated em_queued_call, so an initialization function (except for the args) is not as useful.

Base automatically changed from proxying-cleanup-1-callqueue-pairs to main December 2, 2021 17:40
@tlively tlively merged commit 58dd780 into main Dec 2, 2021
@tlively tlively deleted the init-em-queued-call-args branch December 2, 2021 17:58
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.

3 participants