diff --git a/atom/src/catom.cpp b/atom/src/catom.cpp index ab494b15..242b6fb1 100644 --- a/atom/src/catom.cpp +++ b/atom/src/catom.cpp @@ -102,9 +102,10 @@ CAtom_clear( CAtom* self ) { Py_CLEAR( self->slots[ i ] ); } - if( self->observers ) + if( self->meta.has_observers ) { - self->observers->py_clear(); + ObserverPoolManager::get()->release_pool(self->meta.pool_index); + self->meta.has_observers = false; } } @@ -121,9 +122,9 @@ CAtom_traverse( CAtom* self, visitproc visit, void* arg ) // This was not needed before Python 3.9 (Python issue 35810 and 40217) Py_VISIT(Py_TYPE(self)); #endif - if( self->observers ) + if( self->meta.has_observers ) { - return self->observers->py_traverse( visit, arg ); + return self->observer_pool()->py_traverse( visit, arg ); } return 0; @@ -147,8 +148,6 @@ CAtom_dealloc( CAtom* self ) { PyObject_FREE( self->slots ); } - delete self->observers; - self->observers = 0; Py_TYPE(self)->tp_free( pyobject_cast( self ) ); } @@ -363,8 +362,13 @@ CAtom_sizeof( CAtom* self, PyObject* args ) { Py_ssize_t size = Py_TYPE(self)->tp_basicsize; size += sizeof( PyObject* ) * self->get_slot_count(); - if( self->observers ) - size += self->observers->py_sizeof(); + if( self->meta.has_observers ) + { + size += self->observer_pool()->py_sizeof(); + // account for space in pool manager + // why is sizeof( atom::ObserverPool* ) 16 bytes on macos??? + size += sizeof( atom::ObserverPool* ) + sizeof( uint32_t ); + } return PyLong_FromSsize_t( size ); } @@ -593,9 +597,18 @@ CAtom::observe( PyObject* topic, PyObject* callback, uint8_t change_types ) cppy::ptr callbackptr( wrap_callback( callback ) ); if( !callbackptr ) return false; - if( !observers ) - observers = new ObserverPool(); - observers->add( topicptr, callbackptr, change_types ); + if( !meta.has_observers ) + { + uint32_t index; + if ( !ObserverPoolManager::get()->aquire_pool(index) ) + { + cppy::system_error("Observer pool filled"); + return false; + } + meta.pool_index = index; + meta.has_observers = true; + } + observer_pool()->add( topicptr, callbackptr, change_types ); return true; } @@ -603,11 +616,11 @@ CAtom::observe( PyObject* topic, PyObject* callback, uint8_t change_types ) bool CAtom::unobserve( PyObject* topic, PyObject* callback ) { - if( !observers ) + if( !meta.has_observers ) return true; cppy::ptr topicptr( cppy::incref( topic ) ); cppy::ptr callbackptr( cppy::incref( callback ) ); - observers->remove( topicptr, callbackptr ); + observer_pool()->remove( topicptr, callbackptr ); return true; } @@ -615,10 +628,10 @@ CAtom::unobserve( PyObject* topic, PyObject* callback ) bool CAtom::unobserve( PyObject* topic ) { - if( !observers ) + if( !meta.has_observers ) return true; cppy::ptr topicptr( cppy::incref( topic ) ); - observers->remove( topicptr ); + observer_pool()->remove( topicptr ); return true; } @@ -626,9 +639,10 @@ CAtom::unobserve( PyObject* topic ) bool CAtom::unobserve() { - if( !observers ) + if( !meta.has_observers ) return true; - observers->py_clear(); + ObserverPoolManager::get()->release_pool(meta.pool_index); + meta.has_observers = false; return true; } @@ -636,12 +650,12 @@ CAtom::unobserve() bool CAtom::notify( PyObject* topic, PyObject* args, PyObject* kwargs, uint8_t change_types ) { - if( observers && get_notifications_enabled() ) + if( meta.has_observers && meta.notifications_enabled ) { cppy::ptr topicptr( cppy::incref( topic ) ); cppy::ptr argsptr( cppy::incref( args ) ); cppy::ptr kwargsptr( cppy::xincref( kwargs ) ); - if( !observers->notify( topicptr, argsptr, kwargsptr, change_types ) ) + if( !observer_pool()->notify( topicptr, argsptr, kwargsptr, change_types ) ) return false; } return true; diff --git a/atom/src/catom.h b/atom/src/catom.h index 8ed6b549..37a16e9d 100644 --- a/atom/src/catom.h +++ b/atom/src/catom.h @@ -13,12 +13,6 @@ #define MAX_MEMBER_COUNT ( static_cast( 0xffff ) ) -#define SLOT_COUNT_MASK ( static_cast( 0xffff ) ) -#define FLAGS_MASK ( static_cast( 0xffff0000 ) ) -#define NOTIFICATION_BIT ( static_cast( 1 << 16 ) ) -#define GUARD_BIT ( static_cast( 1 << 17 ) ) -#define ATOMREF_BIT ( static_cast( 1 << 18 ) ) -#define FROZEN_BIT ( static_cast( 1 << 19 ) ) #define catom_cast( o ) ( reinterpret_cast( o ) ) @@ -26,13 +20,23 @@ namespace atom { +PACK(struct CAtomMetadata +{ + uint32_t pool_index; + uint16_t slot_count; + bool notifications_enabled: 1; + bool has_guards: 1; + bool has_atomref: 1; + bool is_frozen: 1; + bool has_observers: 1; // Whether pool index can be used + uint16_t reserverd: 11; +}); struct CAtom { PyObject_HEAD - uint32_t bitfield; // lower 16 == slot count, upper 16 == flags PyObject** slots; - ObserverPool* observers; + CAtomMetadata meta; static PyType_Spec TypeObject_Spec; @@ -42,12 +46,12 @@ struct CAtom uint32_t get_slot_count() { - return bitfield & SLOT_COUNT_MASK; + return meta.slot_count; } void set_slot_count( uint32_t count ) { - bitfield = ( bitfield & FLAGS_MASK ) | ( count & SLOT_COUNT_MASK ); + meta.slot_count = count; } PyObject* get_slot( uint32_t index ) @@ -65,75 +69,69 @@ struct CAtom bool get_notifications_enabled() { - return ( bitfield & NOTIFICATION_BIT ) != 0; + return meta.notifications_enabled; } void set_notifications_enabled( bool enabled ) { - if( enabled ) - bitfield |= NOTIFICATION_BIT; - else - bitfield &= ~NOTIFICATION_BIT; + meta.notifications_enabled = enabled; } bool has_guards() { - return ( bitfield & GUARD_BIT ) != 0; + return meta.has_guards; } void set_has_guards( bool has_guards ) { - if( has_guards ) - bitfield |= GUARD_BIT; - else - bitfield &= ~GUARD_BIT; + meta.has_guards = has_guards; } bool has_atomref() { - return ( bitfield & ATOMREF_BIT ) != 0; + return meta.has_atomref; } void set_has_atomref( bool has_ref ) { - if( has_ref ) - bitfield |= ATOMREF_BIT; - else - bitfield &= ~ATOMREF_BIT; + meta.has_atomref = has_ref; + } + + // Caller must make sure the meta.has_observers is true + ObserverPool* observer_pool() + { + return ObserverPoolManager::get()->access_pool(meta.pool_index); } bool has_observers( PyObject* topic ) { - if( observers ) + if( meta.has_observers ) { cppy::ptr topicptr( cppy::incref( topic ) ); - return observers->has_topic( topicptr ); + return observer_pool()->has_topic( topicptr ); } return false; } bool has_observer( PyObject* topic, PyObject* callback ) { - if( observers ) + if( meta.has_observers ) { cppy::ptr topicptr( cppy::incref( topic ) ); cppy::ptr callbackptr( cppy::incref( callback ) ); - return observers->has_observer( topicptr, callbackptr ); + return observer_pool()->has_observer( topicptr, callbackptr ); } return false; } bool is_frozen() { - return ( bitfield & FROZEN_BIT ) != 0; + return meta.is_frozen; } void set_frozen( bool frozen ) { - if( frozen ) - bitfield |= FROZEN_BIT; - else - bitfield &= ~FROZEN_BIT; + meta.is_frozen = frozen; } bool observe( PyObject* topic, PyObject* callback ) diff --git a/atom/src/observerpool.cpp b/atom/src/observerpool.cpp index f861b852..36c84348 100644 --- a/atom/src/observerpool.cpp +++ b/atom/src/observerpool.cpp @@ -5,6 +5,7 @@ | | The full license is in the file LICENSE, distributed with this software. |----------------------------------------------------------------------------*/ +#include "globalstatic.h" #include "observerpool.h" #include "utils.h" @@ -268,4 +269,41 @@ ObserverPool::py_traverse( visitproc visit, void* arg ) } +GLOBAL_STATIC(ObserverPoolManager, global_observer_pool_manager) + + +ObserverPoolManager* +ObserverPoolManager::get() +{ + return global_observer_pool_manager(); +} + + +bool +ObserverPoolManager::aquire_pool(uint32_t &index) +{ + if ( m_free_slots.empty() ) + { + if ( m_pools.size() >= MAX_OBSERVER_POOL_COUNT) + return false; + // Use emplace back to ensure there is room so release_pool does not allocate + index = m_pools.size(); + m_pools.emplace_back(new ObserverPool); + return true; + } + index = m_free_slots.back(); + m_free_slots.pop_back(); + return true; +} + + +void +ObserverPoolManager::release_pool(uint32_t index) +{ + m_pools.at(index)->clear(); + m_free_slots.emplace_back(index); + // pool size never decreases +} + + } //namespace atom diff --git a/atom/src/observerpool.h b/atom/src/observerpool.h index 7a2954c5..fd162492 100644 --- a/atom/src/observerpool.h +++ b/atom/src/observerpool.h @@ -14,6 +14,7 @@ #include "modifyguard.h" #include "utils.h" +#define MAX_OBSERVER_POOL_COUNT ( static_cast( 0xffffffff ) ) namespace atom { @@ -78,7 +79,7 @@ class ObserverPool int py_traverse( visitproc visit, void* arg ); - void py_clear() + void clear() { m_topics.clear(); // Clearing the vector may cause arbitrary side effects on item @@ -87,6 +88,7 @@ class ObserverPool // destructors run for the old items. std::vector empty; m_observers.swap( empty ); + m_modify_guard = 0; } private: @@ -100,4 +102,28 @@ class ObserverPool }; +class ObserverPoolManager +{ + +public: + static ObserverPoolManager* get(); + + // Aquire a new ObserverPool. If no free spots are available, allocate a new spot + bool aquire_pool(uint32_t &index); + + // Access a pool at the given index + inline ObserverPool* access_pool(uint32_t index) { + return m_pools.at(index); + } + + // Release and free the pool at the given index + void release_pool(uint32_t index); + + ObserverPoolManager() {} + ~ObserverPoolManager() {} +private: + std::vector m_pools; + std::vector m_free_slots; +}; + } // namespace atom diff --git a/tests/test_mem.py b/tests/test_mem.py index ab685e91..2884e97e 100644 --- a/tests/test_mem.py +++ b/tests/test_mem.py @@ -145,3 +145,25 @@ def test_pickle_mem_usage(label): # not sure why I sometimes see a 2 here but the original buggy version # reported values > 50 assert stat.count < 5 + + +def test_atom_sizeof(): + class Point(Atom): + x = Int() + y = Int() + + p = Point() + atom_size = 32 + object_size = 16 + topic_size = 16 + observer_size = 16 + assert sys.getsizeof(p) == atom_size + 2 * object_size + p.observe("x", lambda c: None) + if "darwin" in sys.platform: + pool_ptr_size = 16 + else: + pool_ptr_size = 8 + pool_size = 56 + 4 + pool_ptr_size + assert sys.getsizeof(p) == ( + atom_size + 2 * object_size + pool_size + topic_size + observer_size + )