design: DESTROY and weaken() implementation plan#463
Merged
Conversation
Add comprehensive design document for implementing DESTROY support for blessed objects and weaken/isweak/unweaken for weak references. Key design decisions: - Targeted reference counting on RuntimeBase (only blessed objects with DESTROY, not all objects) - Three-state refCount field: 0=untracked, >0=tracked, MIN_VALUE=destroyed (eliminates need for separate destroyCalled boolean) - Zero overhead for hot path (non-reference assignments) - GC safety net via Cleaner sentinel pattern for escaped references - External WeakRefRegistry (no per-scalar memory cost) Incorporates lessons from PR #450 (eager DESTROY without refcounting caused 20+ test failures and the DestroyManager attempt (proxy reconstruction fundamentally broken). Supersedes dev/design/object_lifecycle.md. Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> EOF )
Updated the design document with findings from bytecode trace analysis: - Revised refCount encoding: four-state (-1=untracked, 0=tracked/zero containers, >0=N containers, MIN_VALUE=destroyed) instead of three-state - Changed bless-time initialization from refCount=1 to refCount=0, since the bless-time RuntimeScalar is a temporary that travels through the return chain without being explicitly cleaned up - Fixed DESTROY ordering: save old referent BEFORE assignment, decrement AFTER assignment (Perl 5 semantics: DESTROY sees new variable state) - Added Section 4A documenting bytecode trace findings (return chain, scope exit, overcounting analysis) - Updated all sections (6-18) for consistency with v3.0 encoding - Clarified Phase numbering (Cleaner is Phase 4, not Phase 5) - Cleaned up default field initialization documentation Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Comprehensive test suite covering all special cases from the design doc. All tests validated against system Perl 5.42. DESTROY tests: - destroy_basic.t (18): scope exit, undef, overwrite, hash delete, multiple refs, blessed array/scalar refs - destroy_edge_cases.t (22): resurrection, re-bless, exception-in-DESTROY, nested DESTROY, local(), overwrite ordering - destroy_return.t (24): single/two/three-boundary returns, implicit return, list context, void context, method chaining - destroy_collections.t (22): array clear/pop/shift/splice, hash clear, nested structures, shared refs, closures - destroy_inheritance.t (10): parent/child/grandparent DESTROY, SUPER, AUTOLOAD fallback, C3 MRO, dynamic @isa weaken tests: - weaken_basic.t (34): isweak, weaken, unweaken, copy-is-strong, different ref types, double weaken, multiple weak refs - weaken_destroy.t (24): DESTROY + weak ref interaction, circular refs, self-ref, tree back-pointers, weak-only ref - weaken_edge_cases.t (42): error on non-ref, overwrite, re-bless, nested structures, resurrection, unweaken restore Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Review fixes: Cleaner sentinel reachability, WeakRefRegistry pinning, missing refcount hooks, VarHandle CAS for thread safety. Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Replaced Cleaner sentinel pattern with stash-walking at shutdown to avoid pinning objects in memory. Global destruction matches Perl 5 semantics for circular references and missed decrements. Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Key changes based on runtime code review: - Fix dynamicRestoreState: don't increment restored value (prevents permanent +1 overcounting that would block DESTROY for local'd globals - Add MortalList defer-decrement mechanism (§6.2A) for delete/pop/shift/splice return values — equivalent to Perl 5's FREETMPS mortal system - Add interpreter scope-exit cleanup (§6.5) with SCOPE_EXIT_CLEANUP opcode (interpreter backend had no equivalent of emitScopeExitNullStores) - Fix pop/shift docs: they return raw elements, not copies - Fix splice location: Operator.java, not RuntimeArray.java - Defer WeakReferenceWrapper to Phase 5 (all bundled module uses are blessed) - Update Phase 2 into 2a/2b/2c, expand test plan and file list Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> EOF )
Key changes: - Scope initial MortalList to RuntimeHash.delete() only. Survey of all blocked modules (POE, DBIx::Class, Moo, Template Toolkit, Log4perl, Data::Printer, Test::Deep) found no real-world pattern needing deterministic DESTROY from pop/shift/splice of blessed objects. - Add MortalList.active boolean gate — false until first bless() into a class with DESTROY. Zero cost for programs without DESTROY. - Defer RuntimeArray.pop/shift and Operator.splice hooks to Phase 5. - Update Phase 2b, Phase 5, test plan, risks, and file list. Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
dab481f to
5518a22
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Design document for implementing DESTROY support for blessed objects and
weaken()/isweak()/unweaken()for weak references. This is a plan document only — no code changes.Key Design Decisions
RuntimeBase— only blessed objects whose class defines DESTROY, not all objectsrefCountfield:-1= untracked,0= tracked with zero counted containers (fresh from bless),>0= N strong references,Integer.MIN_VALUE= destroyed. Eliminates the need for a separatedestroyCalledbooleanrefCount=0at bless time — the bless-time RuntimeScalar is a temporary that travels through the return chain without cleanup; not counting it eliminates overcounting for the common single-boundary return patternset()fast path (int/double/string/undef) is untouched; refcount logic lives only insetLarge()slow pathWeakRefRegistry— no per-scalar memory cost for weak ref trackingrefCountfield fits in existing alignment padding onRuntimeBaseBytecode Trace Findings (v3.0)
Disassembly and interpreter tracing revealed:
addToScalar()->set()->setLarge())return $objwraps the SAME RuntimeScalar in RuntimeList (no copy through return chain)mydeclarations viaaddToScalar(target)->target.set(this)->setLarge()emitScopeExitNullStores— no cleanup formyvariables on return pathemitScopeExitNullStoresIS emitted for all normal scope exits (blocks, loops, if/else)Alternatives Considered
Implementation Phases
refCountfield (default -1), createDestroyDispatchclasssetLarge(),undefine(),scopeExitCleanup(),delete(); initrefCount=0at blessweaken/isweak/unweaken— external registry, proper semantics${^GLOBAL_PHASE}, shutdown hookWhat is Blocked Without This
isweak()always false)Prior Art
ioHolderCountonRuntimeGlob— existing targeted refcount pattern that proves this approach worksTest plan
Generated with Devin