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

Jump to bottom

move libuv callbacks to julia #11728

Merged
merged 1 commit into from
Jun 17, 2015
Merged

move libuv callbacks to julia #11728

merged 1 commit into from
Jun 17, 2015

Conversation

JeffBezanson
Copy link
Sponsor Member

This simplifies the code and avoids some extra argument boxing and dynamic dispatching.

The close callback seems to be the difficult one.

@Keno @vtjnash What is the isopen callback? It doesn't appear to be used; is that right?

@vtjnash
Copy link
Sponsor Member

vtjnash commented Jun 16, 2015

it is (was?) used by jl_uv_close to check the state of the julia side of the object (to prevent double-close, and other such race conditions during the atexit hook)

@vtjnash
Copy link
Sponsor Member

vtjnash commented Jun 16, 2015

💯

@JeffBezanson
Copy link
Sponsor Member Author

It really doesn't look like it's used anymore. Maybe it should be?

@Keno
Copy link
Member

Keno commented Jun 16, 2015

No, I believe it might have been removed when I fixed the error conditions for double-close on the libuv side.

@JeffBezanson
Copy link
Sponsor Member Author

Cool. So that leaves the close callback. Given how it's used in the shutdown process, is it even worth moving to julia? We could special case just that one; very easy since it only takes one argument.

@JeffBezanson
Copy link
Sponsor Member Author

Ah, I also see now that jl_callback_call has a built in check for the first argument being NULL. Do we need that? I assumed non-NULL in the julia callbacks; is that valid?

@JeffBezanson JeffBezanson changed the title WIP: move libuv callbacks to julia move libuv callbacks to julia Jun 17, 2015
@JeffBezanson
move libuv callbacks from C to julia
176ca4f 
`cfunction` makes this wrapper code unnecessary, and in some cases
it is inefficient.

some other minor code deletion
JeffBezanson added a commit that referenced this pull request Jun 17, 2015
@JeffBezanson
Merge pull request #11728 from JuliaLang/jb/libuvcallbacks
1b050f6 
move libuv callbacks to julia
@JeffBezanson JeffBezanson merged commit 1b050f6 into master Jun 17, 2015
Keno
Keno reviewed Jun 17, 2015
View reviewed changes
src/jl_uv.c
}
// getters and setters
DLLEXPORT void *jl_uv_process_data(uv_process_t *p) { return p->data; }
DLLEXPORT void *jl_uv_buf_base(const uv_buf_t *buf) { return buf->base; }
Copy link
Member

Choose a reason for hiding this comment

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

We should be able to do this in julia since we have the correct calling convention now.

@tkelman tkelman deleted the jb/libuvcallbacks branch June 17, 2015 23:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@JeffBezanson @vtjnash @Keno