From d6e88c85e01c02453e4c207afccc355b5544e64a Mon Sep 17 00:00:00 2001 From: Ichthyostega Date: Wed, 2 Nov 2011 01:34:05 +0100 Subject: [PATCH] finish buffer metadata; cover state transitions BufferMetadata complete and working for now --- src/proc/engine/buffer-metadata.hpp | 123 ++++++++++++---- tests/46engine.tests | 2 +- .../proc/engine/buffer-metadata-test.cpp | 137 ++++++++++++++---- 3 files changed, 204 insertions(+), 58 deletions(-) diff --git a/src/proc/engine/buffer-metadata.hpp b/src/proc/engine/buffer-metadata.hpp index f86d1cdaf..9a73ba20a 100644 --- a/src/proc/engine/buffer-metadata.hpp +++ b/src/proc/engine/buffer-metadata.hpp @@ -31,7 +31,7 @@ ** - that overall storage size available within the buffer ** - a pair of custom \em creator and \em destructor functions to use together with this buffer ** - an additional client key to distinguish otherwise otherwise identical client requests - ** These three distinctions are applied in sequence, thus forming a tree with 3 levels. + ** These three distinctions are applied in sequence, thus forming a type tree with 3 levels. ** Only the first distinguishing level (the size) is mandatory. The others are provided, ** because some of the foreseeable buffer providers allow to re-access the data placed ** into the buffer, by assigning an internally managed ID to the buffer. The most @@ -85,15 +85,22 @@ namespace engine { + + /** + * Buffer states + * usable within BufferProvider + * and stored within the metadata + */ enum BufferState - { NIL, - FREE, - LOCKED, - EMITTED, - BLOCKED + { NIL, ///< abstract entry, not yet allocated + FREE, ///< allocated buffer, no longer in use + LOCKED, ///< allocated buffer actively in use + EMITTED, ///< allocated buffer, returned from client + BLOCKED ///< allocated buffer blocked by protocol failure }; + /** * an opaque ID to be used by the BufferProvider implementation. * Typically this will be used, to set apart some pre-registered @@ -131,7 +138,7 @@ namespace engine { }; - namespace { // Helpers for construction within the buffer... + namespace { // (optional) helpers to build an object embedded within the buffer... template inline void @@ -157,6 +164,7 @@ namespace engine { }//(End)placement-new helpers + /** * A pair of functors to maintain a datastructure within the buffer. * TypeHandler describes how to outfit the buffer in a specific way. @@ -263,7 +271,7 @@ namespace engine { - /* === Implementation === */ + /* === Metadata Implementation === */ namespace metadata { @@ -280,6 +288,7 @@ namespace engine { } } + /** * Description of a Buffer-"type". * Key elements will be used to generate hash IDs, @@ -372,6 +381,12 @@ namespace engine { } + LocalKey const& + localKey() const + { + return specifics_; + } + HashVal parentKey() const { return parent_;} operator HashVal() const { return hashID_;} }; @@ -425,7 +440,7 @@ namespace engine { bool isTypeKey() const { - return !bool(buffer_); + return NIL == state_ && !buffer_; } @@ -445,13 +460,15 @@ namespace engine { return buffer_; } + /** Buffer state machine */ Entry& mark (BufferState newState) { __must_not_be_NIL(); - __must_not_be_FREE(); - if ( (state_ == LOCKED && newState == EMITTED) + if ( (state_ == FREE && newState == LOCKED) + ||(state_ == LOCKED && newState == EMITTED) + ||(state_ == LOCKED && newState == BLOCKED) ||(state_ == LOCKED && newState == FREE) ||(state_ == EMITTED && newState == BLOCKED) ||(state_ == EMITTED && newState == FREE) @@ -460,13 +477,24 @@ namespace engine { // allowed transition if (newState == FREE) invokeEmbeddedDtor_and_clear(); + if (newState == LOCKED) + invokeEmbeddedCtor(); state_ = newState; return *this; } - throw error::Fatal ("Invalid buffer state encountered."); + throw error::Fatal ("Invalid buffer state transition."); } + Entry& + lock (void* newBuffer) + { + __must_be_FREE(); + buffer_ = newBuffer; + return mark (LOCKED); + } + + protected: /** @internal maybe invoke a registered TypeHandler's * constructor function, which typically builds some @@ -474,7 +502,7 @@ namespace engine { void invokeEmbeddedCtor() { - REQUIRE (buffer_); + __buffer_required(); if (nontrivial (instanceFunc_)) instanceFunc_.createAttached (buffer_); } @@ -485,7 +513,7 @@ namespace engine { void invokeEmbeddedDtor_and_clear() { - REQUIRE (buffer_); + __buffer_required(); if (nontrivial (instanceFunc_)) instanceFunc_.destroyAttached (buffer_); buffer_ = 0; @@ -510,9 +538,27 @@ namespace engine { "You should invoke markLocked(buffer) prior to access." , LUMIERA_ERROR_LIFECYCLE ); } + + void + __must_be_FREE() const + { + if (FREE != state_) + throw error::Logic ("Buffer already in use" + , LUMIERA_ERROR_LIFECYCLE ); + REQUIRE (!buffer_, "Buffer marked as free, " + "but buffer pointer is set."); + } + + void + __buffer_required() const + { + if (!buffer_) + throw error::Fatal ("Need concrete buffer for any further operations"); + } }; + /** * (Hash)Table to store and manage buffer metadata. * Buffer metadata entries are comprised of a Key part and an extended @@ -617,6 +663,21 @@ namespace engine { + /* ===== Buffer Metadata Frontend ===== */ + + /** + * Registry for managing buffer metadata. + * This is an implementation level service, + * used by the standard BufferProvider implementation. + * Each metadata registry (instance) defines and maintains + * a family of "buffer types"; beyond the buffer storage size, + * the concrete meaning of those types is tied to the corresponding + * BufferProvider implementation and remains opaque. These types are + * represented as hierarchically linked hash keys. The implementation + * may bind a TypeHandler to a specific type, allowing automatic invocation + * of a "constructor" and "destructor" function on each buffer of this type, + * when \em locking or \em freeing the corresponding buffer. + */ class BufferMetadata : boost::noncopyable { @@ -625,8 +686,8 @@ namespace engine { metadata::Table table_; - public: + public: typedef metadata::Key Key; typedef metadata::Entry Entry; @@ -634,8 +695,8 @@ namespace engine { * Such will maintain a family of buffer type entries * and provide a service for storing and retrieving metadata * for concrete buffer entries associated with these types. - * @param implementationID to distinguish families - * of type keys belonging to different registries. + * @param implementationID to distinguish families of + * type keys belonging to different registries. */ BufferMetadata (Literal implementationID) : id_(implementationID) @@ -661,14 +722,10 @@ namespace engine { Key typeKey = trackKey (family_, storageSize); if (nontrivial(instanceFunc)) - { typeKey = trackKey (typeKey, instanceFunc); - } if (nontrivial(specifics)) - { typeKey = trackKey (typeKey, specifics); - } return typeKey; } @@ -680,7 +737,8 @@ namespace engine { return trackKey (parentKey, instanceFunc); } - /** create a sub-type, using a different private-ID (implementation defined) */ + /** create a sub-type, + * using a different private-ID (implementation defined) */ Key key (Key const& parentKey, LocalKey specifics) { @@ -694,7 +752,11 @@ namespace engine { Key const& key (Key const& parentKey, void* concreteBuffer) { - return lock (parentKey,concreteBuffer); + Key derivedKey = Key::forEntry (parentKey, concreteBuffer); + Entry* existing = table_.fetch (derivedKey); + + return existing? *existing + : markLocked (parentKey,concreteBuffer); } /** core operation to access or create a concrete buffer metadata entry. @@ -728,15 +790,19 @@ namespace engine { Entry newEntry(parentKey, concreteBuffer); Entry* existing = table_.fetch (newEntry); + if (existing && onlyNew) throw error::Logic ("Attempt to lock a slot for a new buffer, " - "while actually the old buffer is still locked." + "while actually the old buffer is still locked" + , error::LUMIERA_ERROR_LIFECYCLE ); + if (existing && existing->isLocked()) + throw error::Logic ("Attempt to re-lock a buffer still in use" , error::LUMIERA_ERROR_LIFECYCLE ); if (!existing) return store_and_lock (newEntry); // actual creation else - return *existing; + return existing->lock (concreteBuffer); } /** access the metadata record registered with the given hash key. @@ -812,7 +878,7 @@ namespace engine { - private: + private: template Key @@ -836,7 +902,9 @@ namespace engine { Entry& newEntry = table_.store (metadata); try { - newEntry.invokeEmbeddedCtor(); + newEntry.invokeEmbeddedCtor(); + ENSURE (LOCKED == newEntry.state()); + ENSURE (newEntry.access()); } catch(...) { @@ -845,7 +913,6 @@ namespace engine { } return newEntry; } - }; diff --git a/tests/46engine.tests b/tests/46engine.tests index 293dc00c7..46c1806db 100644 --- a/tests/46engine.tests +++ b/tests/46engine.tests @@ -17,7 +17,7 @@ return: 0 END -PLANNED "buffer metadata and state transitions" BufferMetadata_test < #include #include #include -//#include -//using boost::format; -//using std::string; -//using std::cout; -//using util::for_each; using std::strncpy; using boost::scoped_ptr; using lib::test::randStr; -using util::isnil; using util::isSameObject; +using util::isnil; namespace engine{ namespace test { - -// using lib::AllocationCluster; -// using mobject::session::PEffect; -// using ::engine::BuffHandle; + using lumiera::error::LUMIERA_ERROR_FATAL; using lumiera::error::LUMIERA_ERROR_INVALID; using lumiera::error::LUMIERA_ERROR_LIFECYCLE; @@ -71,8 +56,6 @@ namespace test { HashVal JUST_SOMETHING = 123; void* const SOME_POINTER = &JUST_SOMETHING; -// const uint TEST_SIZE = 1024*1024; -// const uint TEST_ELMS = 20; template @@ -89,6 +72,7 @@ namespace test { + /******************************************************************* * @test verify the properties of the BufferMetadata records used * internally within BufferProvider to attach additional @@ -105,6 +89,7 @@ namespace test { CHECK (ensure_proper_fixture()); verifyBasicProperties(); verifyStandardCase(); + verifyStateMachine(); } @@ -151,8 +136,8 @@ namespace test { CHECK (NIL == m1.state()); CHECK (!meta_->isLocked(key)); - VERIFY_ERROR (LIFECYCLE, m1.mark(EMITTED) ); - VERIFY_ERROR (LIFECYCLE, m1.mark(LOCKED) ); + VERIFY_ERROR (LIFECYCLE, m1.mark(EMITTED)); + VERIFY_ERROR (LIFECYCLE, m1.mark(FREE) ); // now create an active (buffer) entry metadata::Entry& m2 = meta_->markLocked (key, SOME_POINTER); @@ -186,7 +171,7 @@ namespace test { CHECK ( meta_->isKnown(keyX)); CHECK ( meta_->isKnown(key1)); VERIFY_ERROR (LIFECYCLE, m2.access()); - VERIFY_ERROR (LIFECYCLE, m2.mark(LOCKED)); + VERIFY_ERROR (FATAL, m2.mark(LOCKED)); // buffer missing CHECK ( isSameObject (m2, meta_->get(keyX))); // still accessible // release buffer... @@ -202,8 +187,8 @@ namespace test { * @note to get the big picture, please refer to * BufferProviderProtocol_test#verifyStandardCase() * This testcase here performs precisely the metadata related - * operations necessary to carry out the standard case outlined - * in that more high level test. + * operations necessary to carry out the standard case + * outlined on a higher level in the mentioned test. */ void verifyStandardCase() @@ -238,13 +223,19 @@ namespace test { metadata::Entry& r0 = meta_->markLocked(rawBuffType, &rawbuf[0]); metadata::Entry& r1 = meta_->markLocked(rawBuffType, &rawbuf[1]); - + CHECK (LOCKED == f0.state()); CHECK (LOCKED == f1.state()); CHECK (LOCKED == f2.state()); CHECK (LOCKED == r0.state()); CHECK (LOCKED == r1.state()); + CHECK (transaction1 == f0.localKey()); + CHECK (transaction1 == f1.localKey()); + CHECK (transaction1 == f2.localKey()); + CHECK (transaction2 == r0.localKey()); + CHECK (transaction2 == r1.localKey()); + CHECK (f0.access() == frames+0); CHECK (f1.access() == frames+1); @@ -269,6 +260,10 @@ namespace test { accessAs (f1) = testData(2); accessAs (f2) = testData(3); + CHECK (testData(1) == frames[0]); + CHECK (testData(2) == frames[1]); + CHECK (testData(3) == frames[2]); + CHECK (TestFrame::isAlive (f0.access())); CHECK (TestFrame::isAlive (f1.access())); CHECK (TestFrame::isAlive (f2.access())); @@ -283,7 +278,7 @@ namespace test { // client uses the buffers---------------------(End) - f0.mark(FREE); // note: this implicitly invoked the embedded dtor + f0.mark(FREE); // note: implicitly invoking the embedded dtor f1.mark(FREE); f2.mark(FREE); r0.mark(FREE); @@ -296,7 +291,7 @@ namespace test { meta_->release(handle_r0); meta_->release(handle_r1); - CHECK (TestFrame::isDead (&frames[0])); + CHECK (TestFrame::isDead (&frames[0])); // was destroyed implicitly CHECK (TestFrame::isDead (&frames[1])); CHECK (TestFrame::isDead (&frames[2])); @@ -309,9 +304,93 @@ namespace test { CHECK (!meta_->isLocked(handle_f2)); CHECK (!meta_->isLocked(handle_r0)); CHECK (!meta_->isLocked(handle_r1)); + } + + + void + verifyStateMachine() + { + // start with building a type key.... + metadata::Key key = meta_->key(SIZE_A); + CHECK (NIL == meta_->get(key).state()); + CHECK (meta_->get(key).isTypeKey()); + CHECK (!meta_->isLocked(key)); -#if false /////////////////////////////////////////////////////////////////////////////////////////////////////////////UNIMPLEMENTED :: TICKET #834 -#endif /////////////////////////////////////////////////////////////////////////////////////////////////////////////UNIMPLEMENTED :: TICKET #834 + VERIFY_ERROR (LIFECYCLE, meta_->get(key).mark(LOCKED) ); + VERIFY_ERROR (LIFECYCLE, meta_->get(key).mark(EMITTED)); + VERIFY_ERROR (LIFECYCLE, meta_->get(key).mark(BLOCKED)); + VERIFY_ERROR (LIFECYCLE, meta_->get(key).mark(FREE) ); + VERIFY_ERROR (LIFECYCLE, meta_->get(key).mark(NIL) ); + + // now build a concrete buffer entry + metadata::Entry& entry = meta_->markLocked(key, SOME_POINTER); + CHECK (LOCKED == entry.state()); + CHECK (!entry.isTypeKey()); + + CHECK (SOME_POINTER == entry.access()); + + VERIFY_ERROR (FATAL, entry.mark(LOCKED) ); // invalid state transition + VERIFY_ERROR (FATAL, entry.mark(NIL) ); + + entry.mark (EMITTED); // valid transition + CHECK (EMITTED == entry.state()); + CHECK (entry.isLocked()); + + VERIFY_ERROR (FATAL, entry.mark(LOCKED) ); + VERIFY_ERROR (FATAL, entry.mark(EMITTED)); + VERIFY_ERROR (FATAL, entry.mark(NIL) ); + CHECK (EMITTED == entry.state()); + + entry.mark (FREE); + CHECK (FREE == entry.state()); + CHECK (!entry.isLocked()); + CHECK (!entry.isTypeKey()); + + VERIFY_ERROR (LIFECYCLE, entry.access() ); + VERIFY_ERROR (FATAL, entry.mark(LOCKED) ); + VERIFY_ERROR (FATAL, entry.mark(EMITTED)); + VERIFY_ERROR (FATAL, entry.mark(BLOCKED)); + VERIFY_ERROR (FATAL, entry.mark(FREE) ); + VERIFY_ERROR (FATAL, entry.mark(NIL) ); + + // re-use buffer slot, start new lifecycle + void* OTHER_LOCATION = this; + entry.lock (OTHER_LOCATION); + CHECK (LOCKED == entry.state()); + CHECK (entry.isLocked()); + + VERIFY_ERROR (LIFECYCLE, entry.lock(SOME_POINTER)); + + entry.mark (BLOCKED); // go directly to the blocked state + CHECK (BLOCKED == entry.state()); + VERIFY_ERROR (FATAL, entry.mark(LOCKED) ); + VERIFY_ERROR (FATAL, entry.mark(EMITTED) ); + VERIFY_ERROR (FATAL, entry.mark(BLOCKED) ); + VERIFY_ERROR (FATAL, entry.mark(NIL) ); + + CHECK (OTHER_LOCATION == entry.access()); + + entry.mark (FREE); + CHECK (!entry.isLocked()); + VERIFY_ERROR (LIFECYCLE, entry.access() ); + + meta_->lock(key, SOME_POINTER); + CHECK (entry.isLocked()); + + entry.mark (EMITTED); + entry.mark (BLOCKED); + CHECK (BLOCKED == entry.state()); + CHECK (SOME_POINTER == entry.access()); + + // can't discard metadata, need to free first + VERIFY_ERROR (LIFECYCLE, meta_->release(entry) ); + CHECK (meta_->isKnown(entry)); + CHECK (entry.isLocked()); + + entry.mark (FREE); + meta_->release(entry); + CHECK (!meta_->isKnown(entry)); + CHECK ( meta_->isKnown(key)); } };