Skip to content

Save fiber stacks on Fiber directly instead of using Hash #27

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

ysbaddaden
Copy link
Contributor

@ysbaddaden ysbaddaden commented Mar 28, 2025

I noticed a few things:

  1. we can save the fiber stacks directly on Fiber instead of storing each into a different global Hash;
  2. we can safely iterate fibers using Fiber.each (thread safe linked list);
  3. bonus: we don't need to monkey patch Fiber.inactive anymore;
  4. bonus: we don't need track_fiber to be a macro anymore (renamed caller_stack);
  5. bonus: we can list the main fiber of threads, their spawn stack is empty (it could be the stack that led to start the thread) but their yield stack can be interesting.

The above allows us to not need an explicit lock anymore: we're always setting the stack for the current fiber, and we can have a few checks to make sure we can use said stack from another thread.

Which leads me to the last point: we can generate a backtrace from a Slice(Void*) directly (with a few changes) and we can store stacks as Slice(Void*) directly instead of Array(Void*).

The stacks slice is a struct and if its size won't change, then only the pointer will, so we wouldn't need thread synchronization... except for one caveat: the size can change from 0 to N (see comments) so we check for null pointer and size == 0 (to be resilient to any parallel write reorder), before deciding to use the stack.

I'm having some second thoughts about that. Maybe an Array would be safer: we'd merely update a pointer in place 🤔

I still save the stack as Array(Void*) then expose views into the array as Slice(Void*). The underlying buffer size is either 0 or DEPTH + SKIP.

Also allows to generate backtraces for Slice(Void*) directly (no need
for an Array anymore).
@ysbaddaden ysbaddaden added the enhancement New feature or request label Mar 28, 2025
@ysbaddaden ysbaddaden self-assigned this Mar 28, 2025
@ysbaddaden ysbaddaden marked this pull request as draft March 28, 2025 12:00
@ysbaddaden

This comment was marked as outdated.

@ysbaddaden ysbaddaden marked this pull request as ready for review March 28, 2025 12:47
straight-shoota pushed a commit to crystal-lang/crystal that referenced this pull request Apr 17, 2025
…15615)

Internal refactor: allows to generate a decoded backtrace for any given instruction pointer, without requiring an `Array` or an `Exception::CallStack` object.

My use case is crystal-lang/perf-tools#27 where I use slices and where I could merely call this helper method.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Review
Development

Successfully merging this pull request may close these issues.

1 participant