8000 noalloc by craff · Pull Request #12 · lindig/polly · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

noalloc #12

New issue
8000

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

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

noalloc #12

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
cb1d6b1
noalloc
craff Aug 6, 2023
d63e5c3
format with 0.26
craff Aug 7, 2023
d1e7c7e
Bad function name in error
craff Aug 7, 2023
19c9a1c
r < 0
craff Aug 7, 2023
a8d598c
update comment about uerror
craff Aug 7, 2023
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
23 changes: 17 additions & 6 deletions lib/polly.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,19 @@ end

type t = Unix.file_descr (* epoll fd *)

external caml_polly_add : t -> Unix.file_descr -> Events.t -> unit
external caml_raise_unix_error : string -> 'a = "caml_raise_unix_error"
Copy link
Owner

Choose a reason for hiding this comment

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

I don't see why we need this. We can raise an exception simply from OCaml without calling into C first.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because C will get the errno last set by the unix call and position it in the exception.
We probably should use that for eventfd though.

Copy link
Contributor Author
@craff craff Aug 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

By the way, why to you define __FUNCTION__ in eventfd.read and write ?

Copy link
Owner

Choose a reason for hiding this comment

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

__FUNCTION__ is not defined in OCaml 4.08, unfortunately. I could have used __LOC__ instead.


external caml_polly_add : t -> Unix.file_descr -> Events.t -> int
= "caml_polly_add"
[@@noalloc]

external caml_polly_del : t -> Unix.file_descr -> Events.t -> unit
external caml_polly_del : t -> Unix.file_descr -> Events.t -> int
= "caml_polly_del"
[@@noalloc]

external caml_polly_mod : t -> Unix.file_descr -> Events.t -> unit
external caml_polly_mod : t -> Unix.file_descr -> Events.t -> int
= "caml_polly_mod"
[@@noalloc]

external caml_polly_create1 : unit -> t = "caml_polly_create1"

Expand All @@ -70,11 +75,17 @@ let create = caml_polly_create1

let close t = Unix.close t

let add = caml_polly_add
let add t fd e =
let r = caml_polly_add t fd e in
if r < 0 then caml_raise_unix_error "Polly.add"

let del t fd = caml_polly_del t fd Events.empty
let del t fd =
let r = caml_polly_del t fd Events.empty in
if r < 0 then caml_raise_unix_error "Polly.del"
Copy link
Owner

Choose a reason for hiding this comment

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

If we need to preserve errno it should be returned by the C function. With multiple threads there is no guarantee that it is still valid for call_raise_unix_error to pick it up.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The code may work correctly as it is currently written, but it relies on some fragile properties.

errno is typically per-thread (if it wasn't then there would be no safe way to read it in a multi-threaded program).
But if any code would be executed inbetween return from epoll_ctl and reading the errno inside the other C stub then errno may have already been overwritten. This could happen if a signal is delivered to the program, which would be executed the next time OCaml checks for pending signals. The way the code is currently written it doesn't allocate between the call the caml_polly_... and the call to caml_raise..., but that may break easily if the code is changed in the future (e.g. someone adds a debugging statement).
Newer version of OCaml may use an [@poll error] attribute to detect these cases, but it'd be simpler and more robust to do as suggested and return -errno on error.


let upd = caml_polly_mod
let upd t fd e =
let r = caml_polly_mod t fd e in
if r < 0 then caml_raise_unix_error "Polly.mod"

let wait = caml_polly_wait

Expand Down
22 changes: 14 additions & 8 deletions lib/polly_stubs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,19 +35,25 @@ CAMLprim value caml_polly_create1(value val_unit)
CAMLreturn(val_res);
}

/* beware, uerror is a macro in 5.0 and a function in 4.X, the new function
name is caml_uerror. uerror might be deprecated in the future. */
CAMLprim void caml_raise_unix_error(value funname) {
CAMLparam1(funname);
uerror(String_val(funname),Nothing);
}


static value
caml_polly_ctl(value val_epfd, value val_fd, value val_events, int op)
{
CAMLparam3(val_epfd, val_fd, val_events);
struct epoll_event event = {
.events = (uint32_t) Int_val(val_events),
.data.fd = Int_val(val_fd)
};
CAMLparam3(val_epfd, val_fd, val_events);
struct epoll_event event = {
.events = (uint32_t) Int_val(val_events),
.data.fd = Int_val(val_fd)
};

if (epoll_ctl(Int_val(val_epfd), op, Int_val(val_fd), &event) == -1)
uerror(__FUNCTION__, Nothing);

CAMLreturn(Val_unit);
CAMLreturn(Val_int(epoll_ctl(Int_val(val_epfd), op, Int_val(val_fd), &event)));
}

CAMLprim value caml_polly_add(value val_epfd, value val_fd, value val_events)
Expand Down
0