Skip to content

Locals x Effects II#3648

Merged
mshinwell merged 9 commits intomainfrom
locals-effects-rebased
Mar 6, 2025
Merged

Locals x Effects II#3648
mshinwell merged 9 commits intomainfrom
locals-effects-rebased

Conversation

@TheNumbat
Copy link
Copy Markdown
Member

@TheNumbat TheNumbat commented Mar 3, 2025

Rebased version of #2215 with the following additions:

  • I reviewed the code
  • Deletes local state fields from caml_thread_struct that became redundant
  • Frees local arenas in caml_free_stack (previously leaked for every fiber). This also applies to domains and systhreads.
  • All effects tests are enabled
  • Deleted CR about comparing stack->id; there's nothing to compare it with
  • Changed XXX about the initial fiber stack size to a CR

If you run out of memory by creating too many fibers, you can get Fatal error: allocation failure during minor GC when the minor GC tries to malloc a block. This seems unfortunate.

@TheNumbat TheNumbat added runtime effects Relating to algebraic effects locals Local allocation labels Mar 3, 2025
@TheNumbat TheNumbat requested a review from stedolan March 3, 2025 22:21
@TheNumbat TheNumbat force-pushed the locals-effects-rebased branch from e83fad6 to 06b1b23 Compare March 3, 2025 23:42
@stedolan
Copy link
Copy Markdown
Contributor

stedolan commented Mar 4, 2025

This looks good, thanks! Will be good to finally get this patch merged.

@mshinwell mshinwell mentioned this pull request Mar 5, 2025
@mshinwell mshinwell merged commit 26a0b0c into main Mar 6, 2025
22 checks passed
@mshinwell mshinwell deleted the locals-effects-rebased branch March 6, 2025 09:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effects Relating to algebraic effects locals Local allocation runtime

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants