-
Notifications
You must be signed in to change notification settings - Fork 0
Open
Description
Description:
The current CLI implementation relies on injecting the _ferret_profiler instance directly into the builtins module to make it available to the instrumented AST. This is a "magical" and fragile approach that poses significant stability risks:
- Namespace Pollution: It modifies the global python environment, which can conflict with user code or other libraries that might use a similar variable name.
- Shadowing Risks: If a user script accidentally defines a variable named
_ferret_profiler, the instrumentation will crash or behave unpredictably. - Refactoring Difficulty: It creates a hidden, implicit dependency that makes the codebase harder to test and maintain.
Proposed Solution:
Replace the builtins injection with a robust Singleton Accessor Pattern. The FerretInstrumentor should inject explicit import statements into the user's AST that retrieve the active profiler from a safe, module-level accessor.
Tasks:
- Core Refactor (
ferret/core.py):- Implement a module-level global variable
_global_profiler. - Add thread-safe accessor functions:
set_global_profiler(profiler)andget_global_profiler().
- Implement a module-level global variable
- Instrumentor Update (
ferret/cli.py):- Modify
FerretInstrumentorto stop assuming_ferret_profilerexists in builtins. - Update the
_inject_decoratorlogic to insert a localized import at the top of the instrumented function or module (e.g.,from ferret.core import get_global_profiler). - Change the decorator call to use this imported accessor:
@get_global_profiler().measure(...).
- Modify
- Expanded AST Support:
- Extend
FerretInstrumentorto visitast.Lambdanodes and wrap them in a profiling context, as these are currently ignored.
- Extend
Acceptance Criteria:
- Running
ferret run script.pyworks withoutbuiltins._ferret_profilerbeing present in thelocals()orglobals()of the user script. - User scripts that define their own
_ferret_profilervariable do not cause the profiler to crash.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
No labels