From c416bcd77432b544ea7706aaa712a65f8c122ef7 Mon Sep 17 00:00:00 2001 From: Ge Wang Date: Sat, 9 Nov 2024 20:34:51 -0800 Subject: [PATCH 1/4] update ugen refcount 1) remove the special case of shreds remembering/ref-counting UGens created therein 2) remove all reference counting from UGen connection => and =^ 3) depend on the normal garbage collection to clean up UGen references in the code 4) when UGen variables go out of scope (and refcount goes to 0), also disconnect itself from the UGen graph --- src/core/chuck_instr.cpp | 23 +++++++++++++++-------- src/core/chuck_ugen.cpp | 33 ++++++++++++++++++++++----------- 2 files changed, 37 insertions(+), 19 deletions(-) diff --git a/src/core/chuck_instr.cpp b/src/core/chuck_instr.cpp index 7cdece609..7e62e61e1 100644 --- a/src/core/chuck_instr.cpp +++ b/src/core/chuck_instr.cpp @@ -4381,7 +4381,8 @@ t_CKBOOL initialize_object( Chuck_Object * object, Chuck_Type * type, Chuck_VM_S // REFACTOR-2017: added | 1.5.1.5 (ge & andrew) moved here from instantiate_... object->setOriginVM( vm ); // set origin shred for non-ugens | 1.5.1.5 (ge & andrew) moved here from instantiate_... - if( !type->ugen_info && setShredOrigin ) object->setOriginShred( shred ); + // remove dependency on non-ugens | 1.5.4.2 (ge) part of #ugen-refs + if( /* !type->ugen_info && */ setShredOrigin ) object->setOriginShred( shred ); // allocate virtual table object->vtable = new Chuck_VTable; @@ -4410,15 +4411,21 @@ t_CKBOOL initialize_object( Chuck_Object * object, Chuck_Type * type, Chuck_VM_S { // ugen Chuck_UGen * ugen = (Chuck_UGen *)object; + //--------------------------------------- // UGens: needs shred for auto-disconnect when shred is removed // 1.5.1.5 (ge & andrew) moved from instantiate_and_initialize_object() - if( shred ) - { - // add ugen to shred (ref-counted) - shred->add( ugen ); - // add shred to ugen (ref-counted) | 1.5.1.5 (ge) was: ugen->shred = shred; - object->setOriginShred( shred ); - } + // if( shred ) + // { + // // add ugen to shred (ref-counted) + // shred->add( ugen ); + // // add shred to ugen (ref-counted) | 1.5.1.5 (ge) was: ugen->shred = shred; + // object->setOriginShred( shred ); + // } + //--------------------------------------- + // 1.5.4.2 (ge) revisiting the above mechanism, part of #ugen-refs + // now UGens are not ref-counted by shred, is subject to the normal GC, + // and when refcount goes to 0, will remove it self from UGen graph + //--------------------------------------- // set tick if( type->ugen_info->tick ) ugen->tick = type->ugen_info->tick; // added 1.3.0.0 -- tickf for multi-channel tick diff --git a/src/core/chuck_ugen.cpp b/src/core/chuck_ugen.cpp index 3434b5ac5..f3680793b 100644 --- a/src/core/chuck_ugen.cpp +++ b/src/core/chuck_ugen.cpp @@ -245,7 +245,7 @@ void Chuck_UGen::done() assert( this->m_ref_count == 0 ); - // disconnect + // disconnect from UGen graph this->disconnect( TRUE ); m_valid = FALSE; @@ -517,8 +517,11 @@ t_CKBOOL Chuck_UGen::add( Chuck_UGen * src, t_CKBOOL isUpChuck ) // append fa_push_back( m_src_list, m_src_cap, m_num_src, src ); + // increment source count m_num_src++; - src->add_ref(); + // 1.5.4.2 (ge) removed as part of #ugen-refs + // src->add_ref(); + // add from other side src->add_by( this, isUpChuck ); // upchuck @@ -588,7 +591,9 @@ void Chuck_UGen::add_by( Chuck_UGen * dest, t_CKBOOL isUpChuck ) // append fa_push_back( m_dest_list, m_dest_cap, m_num_dest, dest ); - dest->add_ref(); + // 1.5.4.2 (ge) removed as part of #ugen-refs + // dest->add_ref(); + // increment dest count m_num_dest++; // uana @@ -659,7 +664,8 @@ t_CKBOOL Chuck_UGen::remove( Chuck_UGen * src ) m_src_list[--m_num_src] = NULL; src->remove_by( this ); - src->release(); + // 1.5.4.2 (ge) removed as part of #ugen-refs + // src->release(); --k; } } @@ -744,8 +750,8 @@ void Chuck_UGen::remove_by( Chuck_UGen * dest ) for( t_CKUINT j = i+1; j < m_num_dest; j++ ) m_dest_list[j-1] = m_dest_list[j]; - // release - dest->release(); + // 1.5.4.2 (ge) removed as part of #ugen-refs + // dest->release(); // null the last element m_dest_list[--m_num_dest] = NULL; i--; @@ -796,7 +802,7 @@ void Chuck_UGen::remove_all( ) //----------------------------------------------------------------------------- // name: disconnect() -// desc: ... +// desc: disconnect this UGen //----------------------------------------------------------------------------- t_CKBOOL Chuck_UGen::disconnect( t_CKBOOL recursive ) { @@ -809,7 +815,7 @@ t_CKBOOL Chuck_UGen::disconnect( t_CKBOOL recursive ) return inlet()->disconnect( recursive ); } - // remove + // remove from dest while( m_num_dest > 0 ) { // make sure at least one got disconnected @@ -828,9 +834,12 @@ t_CKBOOL Chuck_UGen::disconnect( t_CKBOOL recursive ) // m_dest_list[i]->remove( this ); // m_num_dest = 0; - // disconnect src too? + // recursive? if( recursive ) + { + // disconnect sources, too this->remove_all(); + } return TRUE; } @@ -1379,7 +1388,8 @@ void Chuck_UGen::init_subgraph() Chuck_Object * obj = NULL; // instantiate object for inlet - obj = instantiate_and_initialize_object( this->origin_shred->vm_ref->env()->ckt_ugen, this->origin_shred ); + // 1.5.4.2 (ge) remove special-case refs between UGen and shred; part of #ugen-refs + obj = instantiate_and_initialize_object( this->origin_vm->env()->ckt_ugen, this->origin_vm ); // set as inlet m_inlet = (Chuck_UGen *)obj; // additional reference count @@ -1390,7 +1400,8 @@ void Chuck_UGen::init_subgraph() // CK_SAFE_ADD_REF(this); // instantiate object for outlet - obj = instantiate_and_initialize_object( this->origin_shred->vm_ref->env()->ckt_ugen, this->origin_shred ); + // 1.5.4.2 (ge) remove special-case refs between UGen and shred; part of #ugen-refs + obj = instantiate_and_initialize_object( this->origin_vm->env()->ckt_ugen, this->origin_vm ); // set as outlet m_outlet = (Chuck_UGen *)obj; // additional reference count From f85c479a179a10359e0acba14773d0e336f91b0b Mon Sep 17 00:00:00 2001 From: Ge Wang Date: Sun, 10 Nov 2024 17:50:43 -0800 Subject: [PATCH 2/4] rework ugen clenaup / shred detach mechanism --- src/core/chuck_instr.cpp | 25 ++++++++++++++++--------- src/core/chuck_lang.cpp | 6 ++++++ src/core/chuck_ugen.cpp | 29 ++++++++++++++++++++--------- src/core/chuck_vm.cpp | 36 ++++-------------------------------- 4 files changed, 46 insertions(+), 50 deletions(-) diff --git a/src/core/chuck_instr.cpp b/src/core/chuck_instr.cpp index 7e62e61e1..5ef6c7aa5 100644 --- a/src/core/chuck_instr.cpp +++ b/src/core/chuck_instr.cpp @@ -4381,8 +4381,8 @@ t_CKBOOL initialize_object( Chuck_Object * object, Chuck_Type * type, Chuck_VM_S // REFACTOR-2017: added | 1.5.1.5 (ge & andrew) moved here from instantiate_... object->setOriginVM( vm ); // set origin shred for non-ugens | 1.5.1.5 (ge & andrew) moved here from instantiate_... - // remove dependency on non-ugens | 1.5.4.2 (ge) part of #ugen-refs - if( /* !type->ugen_info && */ setShredOrigin ) object->setOriginShred( shred ); + // change logic: if ugen OR setShredOrigin==TRUE | 1.5.4.2 (ge) part of #ugen-refs + if( type->ugen_info || setShredOrigin ) object->setOriginShred( shred ); // allocate virtual table object->vtable = new Chuck_VTable; @@ -4414,18 +4414,21 @@ t_CKBOOL initialize_object( Chuck_Object * object, Chuck_Type * type, Chuck_VM_S //--------------------------------------- // UGens: needs shred for auto-disconnect when shred is removed // 1.5.1.5 (ge & andrew) moved from instantiate_and_initialize_object() - // if( shred ) - // { - // // add ugen to shred (ref-counted) - // shred->add( ugen ); - // // add shred to ugen (ref-counted) | 1.5.1.5 (ge) was: ugen->shred = shred; - // object->setOriginShred( shred ); - // } //--------------------------------------- // 1.5.4.2 (ge) revisiting the above mechanism, part of #ugen-refs // now UGens are not ref-counted by shred, is subject to the normal GC, // and when refcount goes to 0, will remove it self from UGen graph //--------------------------------------- + if( shred ) + { + // register ugen with originShred; if the shred is preemptively removed + // (e.g., through OTF / Machine.remove()), it will trigger a ugen_detach() + // to disconnect UGens that were created on it... + // 1.5.4.2 (ge) no longer ref-counted as part of #ugen-refs + // FYI the ugen's originShred should be set already (above) for UGens + shred->add( ugen ); + } + //--------------------------------------- // set tick if( type->ugen_info->tick ) ugen->tick = type->ugen_info->tick; // added 1.3.0.0 -- tickf for multi-channel tick @@ -4440,6 +4443,10 @@ t_CKBOOL initialize_object( Chuck_Object * object, Chuck_Type * type, Chuck_VM_S for( t_CKUINT i = 0; i < ugen->m_multi_chan_size; i++ ) { // allocate ugen for each | REFACTOR-2017: added ugen->vm + // NOTE the channels currently are also detached as part of the + // origin shred's ugen_detach() routine when the shred + // is removed; as of 1.5.4.2, however, ugens are no longer + // reference-counted when added to their origin shreds Chuck_Object * obj = instantiate_and_initialize_object( ugen->originVM()->env()->ckt_ugen, ugen->originShred(), ugen->originVM() ); // cast to ugen diff --git a/src/core/chuck_lang.cpp b/src/core/chuck_lang.cpp index c35951471..f83943ba5 100644 --- a/src/core/chuck_lang.cpp +++ b/src/core/chuck_lang.cpp @@ -209,6 +209,12 @@ t_CKBOOL init_class_ugen( Chuck_Env * env, Chuck_Type * type ) func->doc = "get the ugen's buffered operation mode."; if( !type_engine_import_mfun( env, func ) ) goto error; +// // add attachToOriginShred +// func = make_new_mfun( "Shred", "attachToOriginShred", ugen_originShred ); +// func->add_arg( "Shred", "shred" ); +// func->doc = "(Use with care) by default, a UGen is attached to its origin Shred (the Shred the UGen was created on); if that Shred is removed (e.g., by Machine.remove()), it will disconnect its UGens' audio connections. This methods makes it possible for UGen to be attached to either a different Shred or to no Shred if `null` is passed in. The latter makes this UGen a kind of \"free agent\" that is subject to the garbage collector as usual, but no longer to a origin Shred. This is useful "; +// if( !type_engine_import_mfun( env, func ) ) goto error; + // end type_engine_import_class_end( env ); diff --git a/src/core/chuck_ugen.cpp b/src/core/chuck_ugen.cpp index f3680793b..81ebf3240 100644 --- a/src/core/chuck_ugen.cpp +++ b/src/core/chuck_ugen.cpp @@ -236,19 +236,29 @@ void Chuck_UGen::init() //----------------------------------------------------------------------------- // name: done() -// desc: ... +// desc: this function is called when a UGen is about to be deleted, typically +// from the Chuck_UGen's destructor //----------------------------------------------------------------------------- void Chuck_UGen::done() { + // ref count gotta be zero if we get to this function + assert( this->m_ref_count == 0 ); + + // check if we have origin shred reference if( this->origin_shred ) + { + // unregister from origin shred origin_shred->remove( this ); - - assert( this->m_ref_count == 0 ); + // reset/release origin shred reference + setOriginShred( NULL ); + } // disconnect from UGen graph this->disconnect( TRUE ); + // flag m_valid = FALSE; + // reclaim lists fa_done( m_src_list, m_src_cap ); fa_done( m_dest_list, m_dest_cap ); fa_done( m_src_uana_list, m_src_uana_cap ); @@ -261,8 +271,8 @@ void Chuck_UGen::done() // for each multichan reference | 1.5.2.0 for( t_CKUINT i = 0; i < m_multi_chan_size; i++ ) { - // TODO: disconnect? - + // disconnect each channel | 1.5.4.2 (ge) part of #ugen-refs + m_multi_chan[i]->disconnect( TRUE ); // release CK_SAFE_RELEASE( m_multi_chan[i] ); } @@ -273,12 +283,13 @@ void Chuck_UGen::done() // zero out m_multi_chan_size = 0; - // SPENCER: is this okay??? (added 1.3.0.0) - // changed to release | 1.5.2.0 (ge) + // disconnect inlet and outlet (chugraphs) | 1.5.4.2 (ge) part of #ugen-refs + if( m_inlet ) m_inlet->disconnect( TRUE ); + if( m_outlet ) m_outlet->disconnect( TRUE ); + // release the reference held by Chuck_UGen (C++) | 1.5.2.0 (ge) CK_SAFE_RELEASE( m_inlet ); CK_SAFE_RELEASE( m_outlet ); - - // clean up inlet/outlet | 1.5.2.0 (ge) + // clean up inlet/outlet as held by member variables (ChucK) | 1.5.2.0 (ge) if( m_is_subgraph ) ck_subgraph_cleaup_inlet_outlet( this ); // clean up array (added 1.3.0.0) diff --git a/src/core/chuck_vm.cpp b/src/core/chuck_vm.cpp index 9a8096784..cf124c1a6 100644 --- a/src/core/chuck_vm.cpp +++ b/src/core/chuck_vm.cpp @@ -1902,28 +1902,12 @@ t_CKBOOL Chuck_VM_Shred::initialize( Chuck_VM_Code * c, //----------------------------------------------------------------------------- void Chuck_VM_Shred::detach_ugens() { - // check if we have anything in ugen map for this shred - if( !m_ugen_map.size() ) - return; - - // spencer - March 2012 (added 1.3.0.0) - // can't dealloc ugens while they are still keys to a map; - // add reference, store them in a vector, and release them after - // SPENCERTODO: is there a better way to do this???? - std::vector release_v; - release_v.reserve( m_ugen_map.size() ); - // get iterator to our map map::iterator iter = m_ugen_map.begin(); while( iter != m_ugen_map.end() ) { // get the ugen Chuck_UGen * ugen = iter->first; - - // store ref in array for now (added 1.3.0.0) - // NOTE no need to bump reference since now ugen_map ref counts - release_v.push_back(ugen); - // make sure if ugen has an origin shred, it is this one | 1.5.1.5 assert( !ugen->originShred() || ugen->originShred() == this ); // also clear reference to this shred | 1.5.1.5 @@ -1936,17 +1920,6 @@ void Chuck_VM_Shred::detach_ugens() } // clear map m_ugen_map.clear(); - - // loop over vector - for( vector::iterator rvi = release_v.begin(); - rvi != release_v.end(); rvi++ ) - { - // cerr << "RELEASE: " << (void *) *rvi<< endl; - // release it - CK_SAFE_RELEASE( *rvi ); - } - // clear the release vector - release_v.clear(); } @@ -2120,10 +2093,8 @@ t_CKBOOL Chuck_VM_Shred::add( Chuck_UGen * ugen ) return FALSE; // increment reference count (added 1.3.0.0) - CK_SAFE_ADD_REF( ugen ); - - // RUBBISH - // cerr << "vm add ugen: 0x" << hex << (int)ugen << endl; + // remove ref-count from VM-side | 1.5.4.2 (ge) part of #ugen-refs + // CK_SAFE_ADD_REF( ugen ); m_ugen_map[ugen] = ugen; return TRUE; @@ -2145,7 +2116,8 @@ t_CKBOOL Chuck_VM_Shred::remove( Chuck_UGen * ugen ) m_ugen_map.erase( ugen ); // decrement reference count (added 1.3.0.0) - ugen->release(); + // remove ref-count from VM-side | 1.5.4.2 (ge) part of #ugen-refs + // ugen->release(); return TRUE; } From 4e5953b08141d2cbcc9ce025b6f3a32bcef5ac62 Mon Sep 17 00:00:00 2001 From: Ge Wang Date: Mon, 11 Nov 2024 16:09:12 -0800 Subject: [PATCH 3/4] add long-missing fd close in SndBuf dtor --- src/core/ugen_xxx.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/core/ugen_xxx.cpp b/src/core/ugen_xxx.cpp index 195c87fd8..38f72dcb1 100644 --- a/src/core/ugen_xxx.cpp +++ b/src/core/ugen_xxx.cpp @@ -3042,6 +3042,15 @@ struct sndbuf_data ~sndbuf_data() { + // open file descriptor? | 1.5.4.2 (ge) added + if( this->fd ) + { + // close file descriptor + sf_close( this->fd ); + // zero out + this->fd = NULL; + } + CK_SAFE_DELETE_ARRAY( buffer ); if( chunk_map ) From 36248b5d8a14bb101c6e6b7d38b0fa29400a3c6a Mon Sep 17 00:00:00 2001 From: Ge Wang Date: Mon, 11 Nov 2024 18:03:18 -0800 Subject: [PATCH 4/4] update release notes --- VERSIONS | 10 ++++++++++ src/core/ugen_xxx.cpp | 2 +- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/VERSIONS b/VERSIONS index d04f4ef01..aa16f033a 100644 --- a/VERSIONS +++ b/VERSIONS @@ -8,6 +8,16 @@ ChucK VERSIONS log specified paths; instead the specified paths are added to the import system user paths, and the .chug and .ck files within the specified paths can be @imported. +(updated) improved internal garbage collection mechanism for all UGens; + now UGens are garbage collected and disconnected from all its UGen + connections, as soon as it is now longer referenced from code (previously, + this was deferred until the end of origin shred the UGen was created on, + which poses potentially significant memory build-up if a shred repeatedly + instantiated/connected UGens without exiting). Overall, this internal + update should result in improved CPU performance and memory behavior in + various scenarios. +(fixed) long-standing SndBuf issue, especially prevalent on macOS: + "System error : Too many open files." this was due a combination of (added) --auto-load-chugin-path: flag -- this behaves as --chugin-path previously did: chugins in the specified path(s) are auto-loaded; .ck files can be @imported diff --git a/src/core/ugen_xxx.cpp b/src/core/ugen_xxx.cpp index 38f72dcb1..10a0d7bee 100644 --- a/src/core/ugen_xxx.cpp +++ b/src/core/ugen_xxx.cpp @@ -3042,7 +3042,7 @@ struct sndbuf_data ~sndbuf_data() { - // open file descriptor? | 1.5.4.2 (ge) added + // open file descriptor? | 1.5.4.2 (ge) added #ugen-refs if( this->fd ) { // close file descriptor