Skip to content

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

Closed
cgaebel wants to merge 2 commits intoocaml:trunkfrom
cgaebel:noalloc-thread-id
Closed

Make [Thread.id] and [Thread.self] [noalloc].#196
cgaebel wants to merge 2 commits intoocaml:trunkfrom
cgaebel:noalloc-thread-id

Conversation

@cgaebel
Copy link
Copy Markdown

@cgaebel cgaebel 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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done.

@bobot
Copy link
Copy Markdown
Contributor

bobot 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
Copy link
Copy Markdown
Member

gasche 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 noalloc-thread-id branch from 51e3169 to 05a0a19 Compare June 19, 2015 02:38
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 noalloc-thread-id branch from 05a0a19 to 58593a6 Compare June 19, 2015 02:39
@mshinwell
Copy link
Copy Markdown
Contributor

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

@cgaebel
Copy link
Copy Markdown
Author

cgaebel commented Jul 13, 2015

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

@gasche
Copy link
Copy Markdown
Member

gasche 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
lthls pushed a commit to lthls/ocaml that referenced this pull request Jul 1, 2020
lthls pushed a commit to lthls/ocaml that referenced this pull request Sep 23, 2020
lthls pushed a commit to lthls/ocaml that referenced this pull request Sep 23, 2020
lthls pushed a commit to lthls/ocaml that referenced this pull request Sep 24, 2020
chambart pushed a commit to chambart/ocaml-1 that referenced this pull request Sep 9, 2021
EmileTrotignon pushed a commit to EmileTrotignon/ocaml that referenced this pull request Jan 12, 2024
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.

4 participants