Methods for explicit start and stop flamegraph recording

[Envek]

Add Singed.start and Singed.stop to explicitly start and stop flamegraph generation which may be useful for cases when there is no block of code to wrap around like measuring whole application startup etc

Resolves #1
Also see #50

[dduugg]

PR Review: "Methods for explicit start and stop flamegraph recording"

Goal

Resolves #1. Adds Singed.start and Singed.stop for cases where a block-based flamegraph { } wrapper isn't practical — e.g. profiling application boot or measuring across multiple entry points.

---

Implementation

• lib/singed/flamegraph.rb: Refactors record to delegate to new start/stop methods instead of using StackProf.run. Switches from StackProf.run (block form) to StackProf.start + StackProf.stop + StackProf.results. Adds started? predicate backed by @started.
• lib/singed.rb: Adds Singed.start, Singed.stop, Singed.profiling?, and attr_reader :current_flamegraph at the module level. start creates and starts a Flamegraph; stop stops it, saves it, and returns it.
• spec/singed_spec.rb (new): Good coverage of start/stop including the disabled case, double-start guard, nil return when not profiling, and file creation on disk.
• README.md: Documents the new API with a boot-time profiling example.

---

Risk Assessment

Medium risk. The change to Flamegraph#record is a meaningful behavioral shift that affects all existing flamegraph captures.

1. record no longer guards against an already-existing file correctly

Previously:

return yield unless Singed.enabled?
return yield if filename.exist? # file existing means its been captured already

The filename.exist? guard was in record and ran the block without profiling if the file already existed. Now it's in start, which returns false silently — but record calls start and then yield regardless:

def record
  start
  yield
ensure
  stop
end

If start returns false (file exists, disabled, or already started), yield still runs and stop is called. stop returns nil early since started? is false, so no profile is written — which is the correct outcome. But the block's return value is now discarded rather than returned to the caller. This is a behavioral change worth calling out.

2. record no longer returns the block's return value

The old record returned result (the block's return value). The new implementation uses ensure and implicitly returns nil. Flamegraph#record is called from Singed.flamegraph, which is the main user-facing API. If any callers relied on the return value of flamegraph { ... }, they'll silently get nil now.

3. Global StackProf state — only one profiler can run at a time

StackProf.start/stop are global. If Singed.flamegraph { } is called while Singed.start is already active, start returns false (guarded by started?), so the nested call is a no-op — the outer profile continues. This is correct behavior but worth documenting, since users might expect the inner flamegraph block to work independently.

4. Singed.stop calls flamegraph.save but not flamegraph.open

This is intentional for the explicit API (you wouldn't auto-open a browser at boot time), but the README example doesn't mention how to view the result. A note pointing users to flamegraph.open or the output path would help.

5. Singed.output_directory is not reset after Dir.mktmpdir in specs

The around block in spec/singed_spec.rb restores output_directory only if original_output_directory — but mktmpdir is called in before and the temp directory is never cleaned up. Minor leak, but worth using Dir.mktmpdir with a block or adding explicit cleanup.

6. attr_reader :current_flamegraph exposes internal state publicly

This is needed for testing and the README doesn't document it, but making it part of the public API means it's a commitment. Consider whether it should stay or be removed after the profiling? predicate covers the main use case.

---

Summary

Solid feature addition that resolves a long-standing request. The refactor of Flamegraph#record is the riskiest part. Main items before merging:

• Verify callers of flamegraph { } don't rely on the block's return value (the new record drops it)
• Add a note in the README about how to open/locate the saved flamegraph after Singed.stop
• Consider whether current_flamegraph should be public API or kept internal
• Minor: clean up the mktmpdir in specs

[Envek]

1. record no longer guards against an already-existing file correctly

def record
  start
  yield
ensure
  stop
end

<…> But the block's return value is now discarded rather than returned to the caller. <…>

That's not true: yield is the last operator in the method body so its return value becomes method's return value.

See https://docs.ruby-lang.org/en/4.0/syntax/exceptions_rdoc.html#:~:text=Without%20explicit%20return%20in%20the%20ensure%20block

2. record no longer returns the block's return value

Same

3. Global StackProf state — only one profiler can run at a time

Added a note to README.

4. Singed.stop calls flamegraph.save but not flamegraph.open

For me behavior of kernel_ext.rb flamegraph method is a bit surprising. Documented opening part in README.

5. Singed.output_directory is not reset after Dir.mktmpdir in specs

Fixed, though had to change output_directory= writer to handle nil

6. attr_reader :current_flamegraph exposes internal state publicly

Agree. Dropped attr_accessor: there is no real need to expose it.

[dduugg]

Thanks @Envek for the thoughtful reply and for the follow-up changes! You were right on points 1 and 2 — yield before ensure is indeed the return value, appreciate the Ruby docs link. The fixes for the spec cleanup, the nil-safe output_directory= writer, and dropping the attr_reader are all clean. Merging.