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

Make [Thread.id] and [Thread.self] [noalloc]. #196

Closed
wants to merge 2 commits into from

Conversation

Projects
None yet
4 participants
@cgaebel
Copy link

commented Jun 9, 2015

These functions are called every tick of the Async scheduler, and
are the only remaining calls to [caml_c_call] every cycle. It would
be nice to remove them, especially since these functions don't
allocate.

external thread_kill : t -> unit = "thread_kill"
external thread_uncaught_exception : exn -> unit = "thread_uncaught_exception"

external id : t -> int = "thread_id"
external thread_id : t -> int = "thread_id" "noalloc"

This comment has been minimized.

Copy link
@bobot

bobot Jun 16, 2015

Contributor

I'm just picky but could you remove the newline, please? It is not anymore separating exported and not exported external.

This comment has been minimized.

Copy link
@cgaebel

cgaebel Jun 19, 2015

Author

Done.

@bobot

This comment has been minimized.

Copy link
Contributor

commented Jun 16, 2015

Good, and no other functions of the module seems to satisfy the noalloc requirements.

I'm just wondering if we should annotate all the C functions that are declared "noalloc" with a \* noalloc \* in order to not forget that when modifying the C code.

@gasche

This comment has been minimized.

Copy link
Member

commented Jun 16, 2015

@bobot good idea; we could even have it as a silent attribute (defined to be nothing) to use just after CAMLprim -- why is this macro not used in threads/scheduler.c?

@cgaebel cgaebel force-pushed the cgaebel:noalloc-thread-id branch from 51e3169 to 05a0a19 Jun 19, 2015

Make [Thread.id] and [Thread.self] [noalloc].
These functions are called every tick of the Async scheduler, and
are the only remaining calls to [caml_c_call] every cycle. It would
be nice to remove them, especially since these functions don't
allocate.

@cgaebel cgaebel force-pushed the cgaebel:noalloc-thread-id branch from 05a0a19 to 58593a6 Jun 19, 2015

@mshinwell

This comment has been minimized.

Copy link
Contributor

commented Jul 9, 2015

Shouldn't this patch be editing otherlibs/systhreads/, not otherlibs/threads ?

@cgaebel

This comment has been minimized.

Copy link
Author

commented Jul 13, 2015

I've added the changes to otherlibs/systhreads. Why does the patch not apply to otherlibs/threads, also? @mshinwell

@gasche

This comment has been minimized.

Copy link
Member

commented Jul 26, 2015

Merged in trunk, thanks for the patch!

(My understanding is that applying the change to both systhreads and threads is the right thing, and it is what the merged patch does.)

@gasche gasche closed this Jul 26, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.