From f96b24598020d498eb15061b5f179f6323c558f4 Mon Sep 17 00:00:00 2001 From: Lucio Moser Date: Fri, 21 Feb 2014 18:03:57 -0800 Subject: [PATCH 01/11] Making StreamIndexedIO::Node work as MemberData, where it holds a pointer to the global Index and to the low level DirectoryNode. Adding a specific class for DirectoryNode that holds the information loaded from the index. Adding a member m_nodeType in NodeBase to distinguish derived classes without adding virtual methods. --- src/IECore/StreamIndexedIO.cpp | 430 +++++++++++++++++---------------- 1 file changed, 228 insertions(+), 202 deletions(-) diff --git a/src/IECore/StreamIndexedIO.cpp b/src/IECore/StreamIndexedIO.cpp index 32c37a6662..86330fec38 100644 --- a/src/IECore/StreamIndexedIO.cpp +++ b/src/IECore/StreamIndexedIO.cpp @@ -291,24 +291,28 @@ class StreamIndexedIO::StringCache mutable unsigned long m_ioBufferLen; }; -class BaseNode : public RefCounted +class NodeBase { public : + + typedef enum { + Data, + Directory + } NodeType; + + NodeBase( NodeType type ) : m_nodeType(type) {} // name of the node in the current directory IndexedIO::EntryID m_name; - // directory or file - virtual IndexedIO::EntryType entryType() const = 0; - + NodeType m_nodeType; }; -IE_CORE_DECLAREPTR( BaseNode ); - /// Class that represents a Data node -class DataNode : public BaseNode +class DataNode : public NodeBase { public : + DataNode() : NodeBase(NodeBase::Data) {} /// data fields from IndexedIO::Entry IndexedIO::DataType m_dataType; @@ -321,20 +325,13 @@ class DataNode : public BaseNode /// The size of this node's data chunk within the file Imf::Int64 m_size; - - IndexedIO::EntryType entryType() const - { - return IndexedIO::File; - } - }; /// A directory node within an index /// It also represents subindex directory nodes by setting m_subindex at the root and all it's child nodes to true. -class StreamIndexedIO::Node : public BaseNode +class DirectoryNode : public NodeBase { - public: - + public : /// Directory nodes can save it's children to sub-indexes to free resources and reduce the size of the main index. /// Once saved to a subindex, they become read-only. enum SubIndexMode { @@ -343,25 +340,35 @@ class StreamIndexedIO::Node : public BaseNode LoadedSubIndex, }; + DirectoryNode() : NodeBase(NodeBase::Directory), m_subindex(NoSubIndex), m_offset(0), m_parent(0) {} + SubIndexMode m_subindex; /// The offset in the file to this node's subindex block if m_subindex is not NoSubIndex. Imf::Int64 m_offset; - /// A shared pointer to the main file index - StreamIndexedIO::Index* m_idx; - /// A pointer to the parent node in the tree - will be NULL for the root node - Node* m_parent; + DirectoryNode* m_parent; /// Pointers to this node's children (Node or DataNode) - typedef std::map< IndexedIO::EntryID, BaseNodePtr> ChildMap; + typedef std::map< IndexedIO::EntryID, NodeBase*> ChildMap; ChildMap m_children; public: + /// registers a child node in this node + void registerChild( NodeBase* c ); + + void path( IndexedIO::EntryIDList &result ) const; +}; + +// holds the private member data for StreamIndexedIO instance and provides high level access to the directory nodes +class StreamIndexedIO::Node : public RefCounted +{ + public : + /// Construct a new Node in the given index with the given numeric id - Node(StreamIndexedIO::Index* index); + Node(StreamIndexedIO::Index* index, DirectoryNode *dirNode); virtual ~Node(); @@ -369,35 +376,26 @@ class StreamIndexedIO::Node : public BaseNode void childNames( IndexedIO::EntryIDList &names, IndexedIO::EntryType ) const; const IndexedIO::EntryID &name() const; - void path( IndexedIO::EntryIDList &result ) const; bool hasChild( const IndexedIO::EntryID &name ) const; /// Returns a Node or DataNode or NULL if not existent. - BaseNode* child( const IndexedIO::EntryID &name ) const; + NodeBase* child( const IndexedIO::EntryID &name ) const; // Returns the named child node or NULL if not existent. // If loadChildren is true, then it loads the subindex for the child nodes (if applicable). - Node* child( const IndexedIO::EntryID &name, bool loadChildren ) const; + DirectoryNode* child( const IndexedIO::EntryID &name, bool loadChildren ) const; DataNode* dataChild( const IndexedIO::EntryID &name ) const; - Node* addChild( const IndexedIO::EntryID & childName ); + DirectoryNode* addChild( const IndexedIO::EntryID & childName ); DataNode* addDataChild( const IndexedIO::EntryID & childName ); void removeChild( const IndexedIO::EntryID &childName, bool throwException = true ); - IndexedIO::EntryType entryType() const - { - return IndexedIO::Directory; - } - - protected: - - friend class StreamIndexedIO::Index; - - /// registers a child node in this node - void registerChild( BaseNode* c ); + StreamIndexedIO::IndexPtr m_idx; + DirectoryNode *m_node; }; + /// A tree to represent nodes in a filesystem, along with their locations in a file. class StreamIndexedIO::Index : public RefCounted { @@ -412,7 +410,7 @@ class StreamIndexedIO::Index : public RefCounted /// function called right after construction void openStream(); - Node *root() const; + DirectoryNode *root() const; /// Allocate a new chunk of data of the requested size, returning its offset within the file Imf::Int64 allocate( Imf::Int64 sz ); @@ -433,14 +431,17 @@ class StreamIndexedIO::Index : public RefCounted Imf::Int64 writeUniqueData( const char *data, size_t size, bool prefixSize = false ); /// flushes the children of the given directory node to a subindex in the file - void commitNodeToSubIndex( Node *n ); + void commitNodeToSubIndex( DirectoryNode *n ); /// read the subindex that contains the children of the given node - void readNodeFromSubIndex( Node *n ); + void readNodeFromSubIndex( DirectoryNode *n ); protected: - NodePtr m_root; + DirectoryNode *m_root; + + /// we keep all the removed nodes alive until the Index destruction + std::vector< NodeBase * > m_removedNodes; Imf::Int64 m_version; @@ -450,7 +451,7 @@ class StreamIndexedIO::Index : public RefCounted Imf::Int64 m_next; // only used on Version <= 4 - typedef std::vector< BaseNode* > IndexToNodeMap; + typedef std::vector< NodeBase* > IndexToNodeMap; IndexToNodeMap m_indexToNodeMap; typedef std::map< std::pair, Imf::Int64 > HashToDataMap; @@ -481,14 +482,14 @@ class StreamIndexedIO::Index : public RefCounted void addFreePage( Imf::Int64 offset, Imf::Int64 sz ); - void deallocateWalk( BaseNode* n ); + void deallocateWalk( NodeBase* n ); /// Write the index to the file stream Imf::Int64 write(); /// Write the node (and all child nodes) to a stream template < typename F > - void writeNode( Node *n, F &f ); + void writeNode( DirectoryNode *n, F &f ); /// Write the data node to a stream template < typename F > @@ -500,32 +501,25 @@ class StreamIndexedIO::Index : public RefCounted /// Read method used on previous file format versions up to version 4 /// Returns a newly created Node or DataNode. template < typename F > - BaseNode *readNodeV4( F &f ); + NodeBase *readNodeV4( F &f ); /// Replace the contents of this node with data read from a stream. /// Returns a newly created Node or DataNode. template < typename F > - BaseNode *readNode( F &f ); + NodeBase *readNode( F &f ); - void recursiveSetSubIndex( Node *n ); + void recursiveSetSubIndex( DirectoryNode *n ); + static void recursiveNodeDealloc( NodeBase *n ); }; /////////////////////////////////////////////// // -// StreamIndexedIO::Node (begin) +// DirectoryNode (begin) // /////////////////////////////////////////////// -StreamIndexedIO::Node::Node(Index* index) : BaseNode(), m_subindex(NoSubIndex), m_offset(0), m_idx(index), m_parent(0) -{ -} - -StreamIndexedIO::Node::~Node() -{ -} - -void StreamIndexedIO::Node::registerChild( BaseNode* c ) +void DirectoryNode::registerChild( NodeBase* c ) { if ( !c ) { @@ -538,44 +532,65 @@ void StreamIndexedIO::Node::registerChild( BaseNode* c ) throw IOException("StreamIndexedIO: Too many children under the same node!"); } - if ( c->entryType() == IndexedIO::Directory ) + if ( c->m_nodeType == NodeBase::Directory ) { - Node *childNode = static_cast< Node *>(c); + DirectoryNode *childNode = static_cast< DirectoryNode *>(c); if (childNode->m_parent) { throw IOException("StreamIndexedIO: Node already has parent!"); } - childNode->m_parent = this; } + m_children.insert( ChildMap::value_type( c->m_name, c) ); +} - m_children.insert( std::map< IndexedIO::EntryID, BaseNodePtr >::value_type( c->m_name, c) ); +void DirectoryNode::path( IndexedIO::EntryIDList &result ) const +{ + if ( m_parent ) + { + m_parent->path( result ); + result.push_back( m_name ); + } +} + +/////////////////////////////////////////////// +// +// StreamIndexedIO::Node (begin) +// +/////////////////////////////////////////////// + +StreamIndexedIO::Node::Node(Index* index, DirectoryNode *dirNode) : RefCounted(), m_idx(index), m_node(dirNode) +{ +} + +StreamIndexedIO::Node::~Node() +{ } bool StreamIndexedIO::Node::hasChild( const IndexedIO::EntryID &name ) const { - return m_children.find( name ) != m_children.end(); + return m_node->m_children.find( name ) != m_node->m_children.end(); } -BaseNode* StreamIndexedIO::Node::child( const IndexedIO::EntryID &name ) const +NodeBase* StreamIndexedIO::Node::child( const IndexedIO::EntryID &name ) const { - ChildMap::const_iterator cit = m_children.find( name ); - if (cit == m_children.end()) + DirectoryNode::ChildMap::const_iterator cit = m_node->m_children.find( name ); + if (cit == m_node->m_children.end()) { return 0; } - return cit->second.get(); + return cit->second; } -StreamIndexedIO::Node* StreamIndexedIO::Node::child( const IndexedIO::EntryID &name, bool loadChildren ) const +DirectoryNode* StreamIndexedIO::Node::child( const IndexedIO::EntryID &name, bool loadChildren ) const { - Node *n = 0; - BaseNode *p = child(name); + DirectoryNode *n = 0; + NodeBase *p = child(name); if ( p ) { - if ( p->entryType() == IndexedIO::Directory ) + if ( p->m_nodeType == NodeBase::Directory ) { - n = static_cast< Node *>( p ); + n = static_cast< DirectoryNode *>( p ); if ( loadChildren && n->m_subindex ) { @@ -589,10 +604,10 @@ StreamIndexedIO::Node* StreamIndexedIO::Node::child( const IndexedIO::EntryID &n DataNode* StreamIndexedIO::Node::dataChild( const IndexedIO::EntryID &name ) const { DataNode *n = 0; - BaseNode *p = child(name); + NodeBase *p = child(name); if ( p ) { - if ( p->entryType() == IndexedIO::File ) + if ( p->m_nodeType == NodeBase::Data ) { n = static_cast< DataNode *>( p ); } @@ -600,9 +615,9 @@ DataNode* StreamIndexedIO::Node::dataChild( const IndexedIO::EntryID &name ) con return n; } -StreamIndexedIO::Node* StreamIndexedIO::Node::addChild( const IndexedIO::EntryID &childName ) +DirectoryNode* StreamIndexedIO::Node::addChild( const IndexedIO::EntryID &childName ) { - if ( m_subindex ) + if ( m_node->m_subindex ) { throw Exception( "Cannot modify the file at current location! It was already committed to the file." ); } @@ -612,7 +627,7 @@ StreamIndexedIO::Node* StreamIndexedIO::Node::addChild( const IndexedIO::EntryID return 0; } - Node* child = new Node(m_idx); + DirectoryNode* child = new DirectoryNode(); if ( !child ) { throw Exception( "Failed to allocate node!" ); @@ -620,7 +635,7 @@ StreamIndexedIO::Node* StreamIndexedIO::Node::addChild( const IndexedIO::EntryID child->m_name = childName; m_idx->m_stringCache.add( childName ); - registerChild( child ); + m_node->registerChild( child ); m_idx->m_hasChanged = true; @@ -629,7 +644,7 @@ StreamIndexedIO::Node* StreamIndexedIO::Node::addChild( const IndexedIO::EntryID DataNode* StreamIndexedIO::Node::addDataChild( const IndexedIO::EntryID &childName ) { - if ( m_subindex ) + if ( m_node->m_subindex ) { throw Exception( "Cannot modify the file at current location! It was already committed to the file." ); } @@ -649,32 +664,23 @@ DataNode* StreamIndexedIO::Node::addDataChild( const IndexedIO::EntryID &childNa child->m_arrayLength = 0; m_idx->m_stringCache.add( childName ); - registerChild( child ); + m_node->registerChild( child ); m_idx->m_hasChanged = true; return child; } -void StreamIndexedIO::Node::path( IndexedIO::EntryIDList &result ) const -{ - if ( m_parent ) - { - m_parent->path( result ); - result.push_back( m_name ); - } -} - const IndexedIO::EntryID &StreamIndexedIO::Node::name() const { - return m_name; + return m_node->m_name; } void StreamIndexedIO::Node::childNames( IndexedIO::EntryIDList &names ) const { names.clear(); - names.reserve( m_children.size() ); - for ( ChildMap::const_iterator cit = m_children.begin(); cit != m_children.end(); cit++ ) + names.reserve( m_node->m_children.size() ); + for ( DirectoryNode::ChildMap::const_iterator cit = m_node->m_children.begin(); cit != m_node->m_children.end(); cit++ ) { names.push_back( cit->first ); } @@ -683,13 +689,14 @@ void StreamIndexedIO::Node::childNames( IndexedIO::EntryIDList &names ) const void StreamIndexedIO::Node::childNames( IndexedIO::EntryIDList &names, IndexedIO::EntryType type ) const { names.clear(); - names.reserve( m_children.size() ); + names.reserve( m_node->m_children.size() ); bool typeIsDirectory = ( type == IndexedIO::Directory ); - for ( ChildMap::const_iterator cit = m_children.begin(); cit != m_children.end(); cit++ ) + for ( DirectoryNode::ChildMap::const_iterator cit = m_node->m_children.begin(); cit != m_node->m_children.end(); cit++ ) { - bool childIsDirectory = ( cit->second->entryType() == IndexedIO::Directory ); + NodeBase *cc = cit->second; + bool childIsDirectory = ( cc->m_nodeType == NodeBase::Directory ); if ( typeIsDirectory == childIsDirectory ) { names.push_back( cit->first ); @@ -699,8 +706,8 @@ void StreamIndexedIO::Node::childNames( IndexedIO::EntryIDList &names, IndexedIO void StreamIndexedIO::Node::removeChild( const IndexedIO::EntryID &childName, bool throwException ) { - ChildMap::iterator it = m_children.find( childName ); - if ( it == m_children.end() ) + DirectoryNode::ChildMap::iterator it = m_node->m_children.find( childName ); + if ( it == m_node->m_children.end() ) { if (throwException) { @@ -709,11 +716,11 @@ void StreamIndexedIO::Node::removeChild( const IndexedIO::EntryID &childName, bo return; } - BaseNode *child = it->second.get(); + NodeBase *child = it->second; m_idx->deallocateWalk(child); - m_children.erase( it ); + m_node->m_children.erase( it ); } /////////////////////////////////////////////// @@ -744,6 +751,44 @@ StreamIndexedIO::Index::~Index() assert(it->second); delete it->second; } + + // dealloc all nodes, starting from root + recursiveNodeDealloc( m_root ); + + // dealloc removed nodes + for ( std::vector< NodeBase * >::const_iterator it = m_removedNodes.begin(); it != m_removedNodes.end(); it++ ) + { + recursiveNodeDealloc( *it ); + } +} + +void StreamIndexedIO::Index::recursiveNodeDealloc( NodeBase *n ) +{ + if ( !n ) + { + return; + } + switch( n->m_nodeType ) + { + case NodeBase::Directory : + { + DirectoryNode *dn = static_cast< DirectoryNode *>(n); + for (DirectoryNode::ChildMap::const_iterator it = dn->m_children.begin(); it != dn->m_children.end(); ++it) + { + recursiveNodeDealloc( it->second ); + } + delete dn; + break; + } + case NodeBase::Data : + { + DataNode *dn = static_cast< DataNode *>(n); + delete dn; + break; + } + default: + throw Exception("Unknown node type!"); + } } void StreamIndexedIO::Index::flush() @@ -813,15 +858,15 @@ void StreamIndexedIO::Index::openStream() else { // creating a new empty Index - m_root = new Node( this ); + m_root = new DirectoryNode(); m_root->m_name = IndexedIO::rootName; m_hasChanged = true; } } -StreamIndexedIO::Node *StreamIndexedIO::Index::root() const +DirectoryNode *StreamIndexedIO::Index::root() const { - return m_root.get(); + return m_root; } StreamIndexedIO::StringCache &StreamIndexedIO::Index::stringCache() @@ -835,7 +880,7 @@ StreamIndexedIO::StreamFile &StreamIndexedIO::Index::streamFile() const } template < typename F > -BaseNode *StreamIndexedIO::Index::readNodeV4( F &f ) +NodeBase *StreamIndexedIO::Index::readNodeV4( F &f ) { char t; f.read( &t, sizeof(char) ); @@ -888,7 +933,7 @@ BaseNode *StreamIndexedIO::Index::readNodeV4( F &f ) Imf::Int64 parentId; readLittleEndian( f, parentId ); - BaseNode *result = 0; + NodeBase *result = 0; if ( entryType == IndexedIO::File ) { @@ -925,7 +970,7 @@ BaseNode *StreamIndexedIO::Index::readNodeV4( F &f ) } else // Directory { - Node *n = new Node(this); + DirectoryNode *n = new DirectoryNode(); n->m_name = *id; if ( m_version < 2 ) { @@ -940,11 +985,11 @@ BaseNode *StreamIndexedIO::Index::readNodeV4( F &f ) if ( nodeId && parentId != Imath::limits::max() ) { - Node* parent = 0; + DirectoryNode* parent = 0; if ( parentId < m_indexToNodeMap.size() ) { - parent = static_cast< Node * >( m_indexToNodeMap[parentId] ); - if ( parent->entryType() != IndexedIO::Directory ) + parent = static_cast< DirectoryNode * >( m_indexToNodeMap[parentId] ); + if ( parent->m_nodeType != NodeBase::Directory ) { throw IOException("StreamIndexedIO: parent is not a directory!"); } @@ -973,18 +1018,18 @@ BaseNode *StreamIndexedIO::Index::readNodeV4( F &f ) } template < typename F > -BaseNode *StreamIndexedIO::Index::readNode( F &f ) +NodeBase *StreamIndexedIO::Index::readNode( F &f ) { char t; f.read( &t, sizeof(char) ); IndexedIO::EntryType entryType = (IndexedIO::EntryType)t; - StreamIndexedIO::Node::SubIndexMode subindex = StreamIndexedIO::Node::NoSubIndex; + DirectoryNode::SubIndexMode subindex = DirectoryNode::NoSubIndex; if ( t == SUBINDEX_DIR ) { entryType = IndexedIO::Directory; - subindex = StreamIndexedIO::Node::SavedSubIndex; + subindex = DirectoryNode::SavedSubIndex; } Imf::Int64 stringId; @@ -1012,7 +1057,7 @@ BaseNode *StreamIndexedIO::Index::readNode( F &f ) } else if ( entryType == IndexedIO::Directory ) { - Node *n = new Node( this ); + DirectoryNode *n = new DirectoryNode(); n->m_name = m_stringCache.findById( stringId ); n->m_subindex = subindex; @@ -1029,7 +1074,7 @@ BaseNode *StreamIndexedIO::Index::readNode( F &f ) for ( uint32_t c = 0; c < nodeCount; c++ ) { - BaseNode *child = readNode( f ); + NodeBase *child = readNode( f ); n->registerChild( child ); } } @@ -1052,9 +1097,9 @@ void StreamIndexedIO::Index::read( F &f ) if ( m_version >= 5 ) { /// current file format reading - m_root = static_cast< Node *>( readNode( f ) ); + m_root = static_cast< DirectoryNode *>( readNode( f ) ); - if ( m_root->entryType() != IndexedIO::Directory) + if ( m_root->m_nodeType != NodeBase::Directory) { throw Exception( "StreamIndexedIO::Index::read - Root node is not a directory!!" ); } @@ -1071,9 +1116,9 @@ void StreamIndexedIO::Index::read( F &f ) { readNodeV4( f ); } - m_root = static_cast< Node *>( m_indexToNodeMap[0] ); + m_root = static_cast< DirectoryNode *>( m_indexToNodeMap[0] ); - if ( m_root->entryType() != IndexedIO::Directory) + if ( m_root->m_nodeType != NodeBase::Directory) { throw Exception( "StreamIndexedIO::Index::read - Root node is not a directory!!" ); } @@ -1084,7 +1129,7 @@ void StreamIndexedIO::Index::read( F &f ) for (IndexToNodeMap::const_iterator it = m_indexToNodeMap.begin(); it != m_indexToNodeMap.end(); it++ ) { DataNode *n = static_cast< DataNode *>(*it); - if ( !n || n->entryType() != IndexedIO::File ) + if ( !n || n->m_nodeType != NodeBase::Data ) { continue; } @@ -1097,7 +1142,7 @@ void StreamIndexedIO::Index::read( F &f ) throw IOException("StreamIndexedIO: targetNodeId not found"); } DataNode* targetNode = static_cast< DataNode *>(m_indexToNodeMap[targetNodeId]); - if ( targetNode->entryType() != IndexedIO::File ) + if ( targetNode->m_nodeType != NodeBase::Data ) { throw IOException("StreamIndexedIO: targetNode if not of type File!" ); } @@ -1133,7 +1178,7 @@ void StreamIndexedIO::Index::read( F &f ) template < typename F > void StreamIndexedIO::Index::writeDataNode( DataNode *node, F &f ) { - char t = node->entryType(); + char t = IndexedIO::File; f.write( &t, sizeof(char) ); Imf::Int64 id = m_stringCache.find( node->m_name ); @@ -1152,7 +1197,7 @@ void StreamIndexedIO::Index::writeDataNode( DataNode *node, F &f ) } template < typename F > -void StreamIndexedIO::Index::writeNode( Node *node, F &f ) +void StreamIndexedIO::Index::writeNode( DirectoryNode *node, F &f ) { char t = ( node->m_subindex ? SUBINDEX_DIR : IndexedIO::Directory ); f.write( &t, sizeof(char) ); @@ -1168,17 +1213,17 @@ void StreamIndexedIO::Index::writeNode( Node *node, F &f ) { uint32_t nodeCount = node->m_children.size(); writeLittleEndian(f, nodeCount); - for (Node::ChildMap::const_iterator it = node->m_children.begin(); it != node->m_children.end(); ++it) + for (DirectoryNode::ChildMap::const_iterator it = node->m_children.begin(); it != node->m_children.end(); ++it) { - BaseNode *p = it->second.get(); - if ( p->entryType() == IndexedIO::File ) + NodeBase *p = it->second; + if ( p->m_nodeType == NodeBase::Data ) { DataNode *child = static_cast< DataNode * >(p); writeDataNode( child, f ); } else { - Node *child = static_cast< Node * >(p); + DirectoryNode *child = static_cast< DirectoryNode * >(p); writeNode( child, f ); } } @@ -1482,17 +1527,20 @@ Imf::Int64 StreamIndexedIO::Index::writeUniqueData( const char *data, size_t siz return loc; } -void StreamIndexedIO::Index::deallocateWalk( BaseNode* n ) +void StreamIndexedIO::Index::deallocateWalk( NodeBase* n ) { assert(n); - if ( n->entryType() == IndexedIO::Directory ) + // store the pointer for future deallocation + m_removedNodes.push_back( n ); + + if ( n->m_nodeType == NodeBase::Directory ) { - Node *nn = static_cast< Node *>(n); + DirectoryNode *nn = static_cast< DirectoryNode *>(n); - for (Node::ChildMap::const_iterator it = nn->m_children.begin(); it != nn->m_children.end(); ++it) + for (DirectoryNode::ChildMap::const_iterator it = nn->m_children.begin(); it != nn->m_children.end(); ++it) { - deallocateWalk( it->second.get() ); + deallocateWalk( it->second ); } nn->m_children.clear(); @@ -1505,32 +1553,32 @@ void StreamIndexedIO::Index::deallocateWalk( BaseNode* n ) } -void StreamIndexedIO::Index::recursiveSetSubIndex( Node *n ) +void StreamIndexedIO::Index::recursiveSetSubIndex( DirectoryNode *n ) { - n->m_subindex = Node::SavedSubIndex; + n->m_subindex = DirectoryNode::SavedSubIndex; - for (Node::ChildMap::const_iterator it = n->m_children.begin(); it != n->m_children.end(); ++it) + for (DirectoryNode::ChildMap::const_iterator it = n->m_children.begin(); it != n->m_children.end(); ++it) { - Node *childNode = static_cast(it->second.get()); + DirectoryNode *childNode = static_cast(it->second); - if ( childNode->entryType() != IndexedIO::Directory ) + if ( childNode->m_nodeType != NodeBase::Directory ) continue; - if (childNode && childNode->m_subindex == StreamIndexedIO::Node::NoSubIndex ) + if (childNode && childNode->m_subindex == DirectoryNode::NoSubIndex ) { recursiveSetSubIndex( childNode ); } } } -void StreamIndexedIO::Index::commitNodeToSubIndex( Node *n ) +void StreamIndexedIO::Index::commitNodeToSubIndex( DirectoryNode *n ) { if (!n) { return; } - if ( n->m_subindex == Node::NoSubIndex ) + if ( n->m_subindex == DirectoryNode::NoSubIndex ) { MemoryStreamSink sink; io::filtering_ostream compressingStream; @@ -1542,17 +1590,17 @@ void StreamIndexedIO::Index::commitNodeToSubIndex( Node *n ) writeLittleEndian( compressingStream, nodeCount ); - for (Node::ChildMap::const_iterator it = n->m_children.begin(); it != n->m_children.end(); ++it) + for (DirectoryNode::ChildMap::const_iterator it = n->m_children.begin(); it != n->m_children.end(); ++it) { - BaseNode *p = it->second.get(); - if ( p->entryType() == IndexedIO::File ) + NodeBase *p = it->second; + if ( p->m_nodeType == NodeBase::Data ) { DataNode *childNode = static_cast< DataNode *>(p); writeDataNode( childNode, compressingStream ); } else { - Node *childNode = static_cast< Node *>(p); + DirectoryNode *childNode = static_cast< DirectoryNode *>(p); writeNode( childNode, compressingStream ); } @@ -1576,9 +1624,9 @@ void StreamIndexedIO::Index::commitNodeToSubIndex( Node *n ) } } -void StreamIndexedIO::Index::readNodeFromSubIndex( Node *n ) +void StreamIndexedIO::Index::readNodeFromSubIndex( DirectoryNode *n ) { - if ( n->m_subindex == Node::NoSubIndex ) + if ( n->m_subindex == DirectoryNode::NoSubIndex ) { return; } @@ -1586,7 +1634,7 @@ void StreamIndexedIO::Index::readNodeFromSubIndex( Node *n ) /// guarantees thread safe access to the file and also to the m_subindex variable StreamFile::MutexLock lock( m_stream->mutex() ); - if ( n->m_subindex == Node::LoadedSubIndex ) + if ( n->m_subindex == DirectoryNode::LoadedSubIndex ) { return; } @@ -1611,12 +1659,12 @@ void StreamIndexedIO::Index::readNodeFromSubIndex( Node *n ) for ( uint32_t i = 0; i < nodeCount; i++ ) { - BaseNode *child = readNode( decompressingStream ); + NodeBase *child = readNode( decompressingStream ); n->registerChild( child ); } /// mark the node as loaded from subindex - n->m_subindex = Node::LoadedSubIndex; + n->m_subindex = DirectoryNode::LoadedSubIndex; } /////////////////////////////////////////////// @@ -1763,20 +1811,16 @@ StreamIndexedIO::StreamIndexedIO() : m_node(0) StreamIndexedIO::StreamIndexedIO( StreamIndexedIO::Node &node ) { m_node = &node; - // we add a reference to the index - m_node->m_idx->addRef(); } void StreamIndexedIO::open( StreamFilePtr file, const IndexedIO::EntryIDList &root ) { IndexPtr newIndex = new Index( file ); newIndex->openStream(); - m_node = newIndex->root(); + m_node = new StreamIndexedIO::Node( newIndex, newIndex->root() ); setRoot( root ); assert( m_node ); - - // we add a reference to the index - newIndex->addRef(); + assert( m_node->dirNode() ); // \todo Currently in Append mode, the nodes lazily loaded will not be editable. // In order to fully support it, we should probably read all indexes in memory, @@ -1786,24 +1830,7 @@ void StreamIndexedIO::open( StreamFilePtr file, const IndexedIO::EntryIDList &ro StreamIndexedIO::~StreamIndexedIO() { - if ( m_node.get() ) - { - // remove our reference to the index, it's destructor triggers the - // flush to disk. - m_node->m_idx->removeRef(); - - // \todo Free some memory for committed directories in read-only mode: - /// Should we deallocate the tree when the StreamIndexedIO for the root dies? - /// m_node = 0; - /// Lock read mutex - /// if ( m_node->refCount() == 1 ) - /// only parent pointing to it... - /// if ( m_node->n_subindex && m_node->n_offset ) - // // check if all the children has refCount == 1 - // if ( childrenRefCount is one ) - // m_node->children.clear() - // m_node->m_subindex = Node::SavedSubIndex - } + // \todo Consider a mechanism for deallocating sub-indexes } void StreamIndexedIO::setRoot( const IndexedIO::EntryIDList &root ) @@ -1811,12 +1838,12 @@ void StreamIndexedIO::setRoot( const IndexedIO::EntryIDList &root ) IndexedIO::EntryIDList::const_iterator t = root.begin(); for ( ; t != root.end(); t++ ) { - NodePtr childNode = m_node->child( *t, true ); + DirectoryNode* childNode = m_node->child( *t, true ); if ( !childNode ) { break; } - m_node = childNode; + m_node->m_node = childNode; } bool found = ( t == root.end() ); @@ -1838,12 +1865,12 @@ void StreamIndexedIO::setRoot( const IndexedIO::EntryIDList &root ) { for ( ; t != root.end(); t++ ) { - NodePtr childNode = m_node->addChild( *t ); + DirectoryNode *childNode = m_node->addChild( *t ); if ( !childNode ) { throw IOException( "StreamIndexedIO: Cannot create entry '" + (*t).value() + "'" ); } - m_node = childNode; + m_node->m_node = childNode; } } } @@ -1873,7 +1900,7 @@ const IndexedIO::EntryID &StreamIndexedIO::currentEntryId() const void StreamIndexedIO::path( IndexedIO::EntryIDList &result ) const { result.clear(); - m_node->path(result); + m_node->m_node->path(result); } void StreamIndexedIO::entryIds( IndexedIO::EntryIDList &names ) const @@ -1895,7 +1922,7 @@ bool StreamIndexedIO::hasEntry( const IndexedIO::EntryID &name ) const IndexedIOPtr StreamIndexedIO::subdirectory( const IndexedIO::EntryID &name, IndexedIO::MissingBehaviour missingBehaviour ) { assert( m_node ); - Node *childNode = m_node->child( name, true ); + DirectoryNode *childNode = m_node->child( name, true ); if ( !childNode ) { if ( missingBehaviour == IndexedIO::CreateIfMissing ) @@ -1916,14 +1943,15 @@ IndexedIOPtr StreamIndexedIO::subdirectory( const IndexedIO::EntryID &name, Inde throw IOException( "StreamIndexedIO: Could not find child '" + name.value() + "'" ); } } - return duplicate(*childNode); + StreamIndexedIO::Node *newNode = new StreamIndexedIO::Node( m_node->m_idx, childNode ); + return duplicate(*newNode); } ConstIndexedIOPtr StreamIndexedIO::subdirectory( const IndexedIO::EntryID &name, IndexedIO::MissingBehaviour missingBehaviour ) const { readable(name); assert( m_node ); - Node *childNode = m_node->child( name, true ); + DirectoryNode *childNode = m_node->child( name, true ); if ( !childNode ) { if ( missingBehaviour == IndexedIO::NullIfMissing ) @@ -1936,7 +1964,8 @@ ConstIndexedIOPtr StreamIndexedIO::subdirectory( const IndexedIO::EntryID &name, } throw IOException( "StreamIndexedIO: Could not find child '" + name.value() + "'" ); } - return duplicate(*childNode); + StreamIndexedIO::Node *newNode = new StreamIndexedIO::Node( m_node->m_idx, childNode ); + return duplicate(*newNode); } IndexedIOPtr StreamIndexedIO::createSubdirectory( const IndexedIO::EntryID &name ) @@ -1947,12 +1976,13 @@ IndexedIOPtr StreamIndexedIO::createSubdirectory( const IndexedIO::EntryID &name throw IOException( "Child '" + name.value() + "' already exists!" ); } writable( name ); - Node *childNode = m_node->addChild( name ); + DirectoryNode *childNode = m_node->addChild( name ); if ( !childNode ) { throw IOException( "StreamIndexedIO: Could not insert child '" + name.value() + "'" ); } - return duplicate(*childNode); + StreamIndexedIO::Node *newNode = new StreamIndexedIO::Node( m_node->m_idx, childNode ); + return duplicate(*newNode); } void StreamIndexedIO::remove( const IndexedIO::EntryID &name ) @@ -1965,7 +1995,7 @@ void StreamIndexedIO::removeAll( ) { assert( m_node ); - if ( m_node->m_subindex ) + if ( m_node->m_node->m_subindex ) { throw Exception( "Cannot modify the file at current location! It was already committed to the file." ); } @@ -1983,7 +2013,7 @@ void StreamIndexedIO::remove( const IndexedIO::EntryID &name, bool throwIfNonExi assert( m_node ); writable(name); - if ( m_node->m_subindex ) + if ( m_node->m_node->m_subindex ) { throw Exception( "Cannot modify the file at current location! It was already committed to the file." ); } @@ -1996,14 +2026,14 @@ IndexedIO::Entry StreamIndexedIO::entry(const IndexedIO::EntryID &name) const assert( m_node ); readable(name); - BaseNode* node = m_node->child( name ); + NodeBase* node = m_node->child( name ); if (!node) { throw IOException( "StreamIndexedIO: Entry not found '" + name.value() + "'" ); } - if ( node->entryType() == IndexedIO::File ) + if ( node->m_nodeType == NodeBase::Data ) { DataNode *dn = static_cast< DataNode * >(node); return IndexedIO::Entry( dn->m_name, IndexedIO::File, dn->m_dataType, dn->m_arrayLength ); @@ -2015,47 +2045,43 @@ IndexedIO::Entry StreamIndexedIO::entry(const IndexedIO::EntryID &name) const IndexedIOPtr StreamIndexedIO::parentDirectory() { assert( m_node ); - Node *parentNode = m_node->m_parent; + DirectoryNode *parentNode = m_node->m_node->m_parent; if ( !parentNode ) { return NULL; } - return duplicate(*parentNode); + StreamIndexedIO::Node *newNode = new StreamIndexedIO::Node( m_node->m_idx, parentNode ); + return duplicate(*newNode); } ConstIndexedIOPtr StreamIndexedIO::parentDirectory() const { assert( m_node ); - Node* parentNode = m_node->m_parent; + DirectoryNode* parentNode = m_node->m_node->m_parent; if ( !parentNode ) { return NULL; } - return duplicate(*parentNode); + StreamIndexedIO::Node *newNode = new StreamIndexedIO::Node( m_node->m_idx, parentNode ); + return duplicate(*newNode); } IndexedIOPtr StreamIndexedIO::directory( const IndexedIO::EntryIDList &path, IndexedIO::MissingBehaviour missingBehaviour ) { // from the root go to the path - Node* root = m_node; - Node* parentNode = root->m_parent; - while (parentNode) - { - root = parentNode; - parentNode = root->m_parent; - } - Node* node = root; + StreamIndexedIO::Node *newNode = new StreamIndexedIO::Node( m_node->m_idx, m_node->m_idx->root() ); + for ( IndexedIO::EntryIDList::const_iterator pIt = path.begin(); pIt != path.end(); pIt++ ) { const IndexedIO::EntryID &name = *pIt; - Node* childNode = node->child( name, true ); + DirectoryNode* childNode = newNode->child( name, true ); if ( !childNode ) { if ( missingBehaviour == IndexedIO::CreateIfMissing ) { writable( name ); - childNode = node->addChild( name ); + childNode = newNode->addChild( name ); if ( !childNode ) { throw IOException( "StreamIndexedIO: Could not insert child '" + name.value() + "'" ); @@ -2070,9 +2096,9 @@ IndexedIOPtr StreamIndexedIO::directory( const IndexedIO::EntryIDList &path, Ind throw IOException( "StreamIndexedIO: Could not find child '" + name.value() + "'" ); } } - node = childNode; + newNode->m_node = childNode; } - return duplicate(*node); + return duplicate(*newNode); } ConstIndexedIOPtr StreamIndexedIO::directory( const IndexedIO::EntryIDList &path, IndexedIO::MissingBehaviour missingBehaviour ) const @@ -2082,7 +2108,7 @@ ConstIndexedIOPtr StreamIndexedIO::directory( const IndexedIO::EntryIDList &path void StreamIndexedIO::commit() { - m_node->m_idx->commitNodeToSubIndex( m_node ); + m_node->m_idx->commitNodeToSubIndex( m_node->m_node ); } void StreamIndexedIO::write(const IndexedIO::EntryID &name, const InternedString *x, unsigned long arrayLength) From 5d82c15ff4b44b29ed2b36e1e98314971ca997f9 Mon Sep 17 00:00:00 2001 From: Lucio Moser Date: Fri, 21 Feb 2014 20:30:58 -0800 Subject: [PATCH 02/11] Replacing the std::map to a sorted vector and sorting by the m_name field. Using char instead of the enums in the structs to compact members in one word. Original struct sizes were StreamIndexedIO::Node 104 bytes and DataNode was 56. Current struct sizes are DirectoryNode 56 and DataNode 40 bytes. --- src/IECore/StreamIndexedIO.cpp | 90 +++++++++++++++++++++++++--------- 1 file changed, 68 insertions(+), 22 deletions(-) diff --git a/src/IECore/StreamIndexedIO.cpp b/src/IECore/StreamIndexedIO.cpp index 86330fec38..3f48532fdb 100644 --- a/src/IECore/StreamIndexedIO.cpp +++ b/src/IECore/StreamIndexedIO.cpp @@ -300,12 +300,21 @@ class NodeBase Directory } NodeType; + NodeBase( IndexedIO::EntryID name ) : m_name(name) {} + NodeBase( NodeType type ) : m_nodeType(type) {} // name of the node in the current directory IndexedIO::EntryID m_name; - NodeType m_nodeType; + // using char instead of enum to compact members in one word + char m_nodeType; + + static bool compareNames(const NodeBase* a, const NodeBase* b) + { + return a->m_name < b->m_name; + } + }; /// Class that represents a Data node @@ -325,6 +334,7 @@ class DataNode : public NodeBase /// The size of this node's data chunk within the file Imf::Int64 m_size; + }; /// A directory node within an index @@ -340,9 +350,10 @@ class DirectoryNode : public NodeBase LoadedSubIndex, }; - DirectoryNode() : NodeBase(NodeBase::Directory), m_subindex(NoSubIndex), m_offset(0), m_parent(0) {} + DirectoryNode() : NodeBase(NodeBase::Directory), m_subindex(NoSubIndex), m_sortedChildren(false), m_offset(0), m_parent(0) {} - SubIndexMode m_subindex; + char m_subindex; // using char instead of enum to compact members in one word + bool m_sortedChildren; // same as above /// The offset in the file to this node's subindex block if m_subindex is not NoSubIndex. Imf::Int64 m_offset; @@ -350,12 +361,34 @@ class DirectoryNode : public NodeBase /// A pointer to the parent node in the tree - will be NULL for the root node DirectoryNode* m_parent; - /// Pointers to this node's children (Node or DataNode) - typedef std::map< IndexedIO::EntryID, NodeBase*> ChildMap; + /// Sorted list of node's children (DirectoryNode or DataNode) + typedef std::vector< NodeBase* > ChildMap; ChildMap m_children; public: + inline void sortChildren() + { + if ( !m_sortedChildren ) + { + std::sort( m_children.begin(), m_children.end(), NodeBase::compareNames ); + m_sortedChildren = true; + } + } + + template< typename T > + bool findChild( IndexedIO::EntryID name, T &it ) + { + sortChildren(); + NodeBase search(name); + it = std::lower_bound(m_children.begin(), m_children.end(), &search, NodeBase::compareNames ); + if ( it == m_children.end() ) + { + return false; + } + return ( (*it)->m_name == name ); + } + /// registers a child node in this node void registerChild( NodeBase* c ); @@ -541,7 +574,8 @@ void DirectoryNode::registerChild( NodeBase* c ) } childNode->m_parent = this; } - m_children.insert( ChildMap::value_type( c->m_name, c) ); + m_children.push_back( c ); + m_sortedChildren = false; } void DirectoryNode::path( IndexedIO::EntryIDList &result ) const @@ -569,17 +603,18 @@ StreamIndexedIO::Node::~Node() bool StreamIndexedIO::Node::hasChild( const IndexedIO::EntryID &name ) const { - return m_node->m_children.find( name ) != m_node->m_children.end(); + DirectoryNode::ChildMap::const_iterator cit; + return m_node->findChild( name, cit ); } NodeBase* StreamIndexedIO::Node::child( const IndexedIO::EntryID &name ) const { - DirectoryNode::ChildMap::const_iterator cit = m_node->m_children.find( name ); - if (cit == m_node->m_children.end()) + DirectoryNode::ChildMap::const_iterator cit; + if ( m_node->findChild( name, cit ) ) { - return 0; + return *cit; } - return cit->second; + return 0; } DirectoryNode* StreamIndexedIO::Node::child( const IndexedIO::EntryID &name, bool loadChildren ) const @@ -682,7 +717,7 @@ void StreamIndexedIO::Node::childNames( IndexedIO::EntryIDList &names ) const names.reserve( m_node->m_children.size() ); for ( DirectoryNode::ChildMap::const_iterator cit = m_node->m_children.begin(); cit != m_node->m_children.end(); cit++ ) { - names.push_back( cit->first ); + names.push_back( (*cit)->m_name ); } } @@ -695,19 +730,19 @@ void StreamIndexedIO::Node::childNames( IndexedIO::EntryIDList &names, IndexedIO for ( DirectoryNode::ChildMap::const_iterator cit = m_node->m_children.begin(); cit != m_node->m_children.end(); cit++ ) { - NodeBase *cc = cit->second; + NodeBase *cc = *cit; bool childIsDirectory = ( cc->m_nodeType == NodeBase::Directory ); if ( typeIsDirectory == childIsDirectory ) { - names.push_back( cit->first ); + names.push_back( (*cit)->m_name ); } } } void StreamIndexedIO::Node::removeChild( const IndexedIO::EntryID &childName, bool throwException ) { - DirectoryNode::ChildMap::iterator it = m_node->m_children.find( childName ); - if ( it == m_node->m_children.end() ) + DirectoryNode::ChildMap::iterator it; + if ( !m_node->findChild( childName, it ) ) { if (throwException) { @@ -716,7 +751,7 @@ void StreamIndexedIO::Node::removeChild( const IndexedIO::EntryID &childName, bo return; } - NodeBase *child = it->second; + NodeBase *child = *it; m_idx->deallocateWalk(child); @@ -775,7 +810,7 @@ void StreamIndexedIO::Index::recursiveNodeDealloc( NodeBase *n ) DirectoryNode *dn = static_cast< DirectoryNode *>(n); for (DirectoryNode::ChildMap::const_iterator it = dn->m_children.begin(); it != dn->m_children.end(); ++it) { - recursiveNodeDealloc( it->second ); + recursiveNodeDealloc( *it ); } delete dn; break; @@ -1077,6 +1112,8 @@ NodeBase *StreamIndexedIO::Index::readNode( F &f ) NodeBase *child = readNode( f ); n->registerChild( child ); } + // force sorting all children so that read-only is multi-threaded + n->sortChildren(); } return n; } @@ -1123,6 +1160,15 @@ void StreamIndexedIO::Index::read( F &f ) throw Exception( "StreamIndexedIO::Index::read - Root node is not a directory!!" ); } + // force sorting all children so that read-only is multi-threaded + for (IndexToNodeMap::const_iterator it = m_indexToNodeMap.begin(); it != m_indexToNodeMap.end(); it++ ) + { + if ( (*it)->m_nodeType == NodeBase::Directory ) + { + static_cast< DirectoryNode * >(*it)->sortChildren(); + } + } + if ( m_version == 4 ) { // In Version 4, symlinks have to get the Entry information from their target nodes. @@ -1215,7 +1261,7 @@ void StreamIndexedIO::Index::writeNode( DirectoryNode *node, F &f ) writeLittleEndian(f, nodeCount); for (DirectoryNode::ChildMap::const_iterator it = node->m_children.begin(); it != node->m_children.end(); ++it) { - NodeBase *p = it->second; + NodeBase *p = *it; if ( p->m_nodeType == NodeBase::Data ) { DataNode *child = static_cast< DataNode * >(p); @@ -1540,7 +1586,7 @@ void StreamIndexedIO::Index::deallocateWalk( NodeBase* n ) for (DirectoryNode::ChildMap::const_iterator it = nn->m_children.begin(); it != nn->m_children.end(); ++it) { - deallocateWalk( it->second ); + deallocateWalk( *it ); } nn->m_children.clear(); @@ -1559,7 +1605,7 @@ void StreamIndexedIO::Index::recursiveSetSubIndex( DirectoryNode *n ) for (DirectoryNode::ChildMap::const_iterator it = n->m_children.begin(); it != n->m_children.end(); ++it) { - DirectoryNode *childNode = static_cast(it->second); + DirectoryNode *childNode = static_cast(*it); if ( childNode->m_nodeType != NodeBase::Directory ) continue; @@ -1592,7 +1638,7 @@ void StreamIndexedIO::Index::commitNodeToSubIndex( DirectoryNode *n ) for (DirectoryNode::ChildMap::const_iterator it = n->m_children.begin(); it != n->m_children.end(); ++it) { - NodeBase *p = it->second; + NodeBase *p = *it; if ( p->m_nodeType == NodeBase::Data ) { DataNode *childNode = static_cast< DataNode *>(p); From f96ed267bc767681e9b7935fef06837ec476d9a4 Mon Sep 17 00:00:00 2001 From: Lucio Moser Date: Fri, 21 Feb 2014 21:03:16 -0800 Subject: [PATCH 03/11] Changing StreamIndexedIO member from intrusive pointer to raw pointer. This does not affect the class size and should not break binary compatibility. As a consequence the Node class reduced from 32 bytes to 16 bytes (not derived from RefCounted anymore). --- include/IECore/StreamIndexedIO.h | 5 +++-- src/IECore/StreamIndexedIO.cpp | 14 ++++++-------- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/include/IECore/StreamIndexedIO.h b/include/IECore/StreamIndexedIO.h index 474c1c1415..19a5507ff7 100644 --- a/include/IECore/StreamIndexedIO.h +++ b/include/IECore/StreamIndexedIO.h @@ -255,10 +255,11 @@ class StreamIndexedIO : public IndexedIO StreamFile &streamFile() const; - NodePtr m_node; - private : + // \todo Consider exposing Node to avoid too much memory fragmentation and save one raw pointer. + Node *m_node; + void setRoot( const IndexedIO::EntryIDList &root ); }; diff --git a/src/IECore/StreamIndexedIO.cpp b/src/IECore/StreamIndexedIO.cpp index 3f48532fdb..06db1a2129 100644 --- a/src/IECore/StreamIndexedIO.cpp +++ b/src/IECore/StreamIndexedIO.cpp @@ -396,15 +396,13 @@ class DirectoryNode : public NodeBase }; // holds the private member data for StreamIndexedIO instance and provides high level access to the directory nodes -class StreamIndexedIO::Node : public RefCounted +class StreamIndexedIO::Node { public : /// Construct a new Node in the given index with the given numeric id Node(StreamIndexedIO::Index* index, DirectoryNode *dirNode); - virtual ~Node(); - void childNames( IndexedIO::EntryIDList &names ) const; void childNames( IndexedIO::EntryIDList &names, IndexedIO::EntryType ) const; @@ -593,11 +591,7 @@ void DirectoryNode::path( IndexedIO::EntryIDList &result ) const // /////////////////////////////////////////////// -StreamIndexedIO::Node::Node(Index* index, DirectoryNode *dirNode) : RefCounted(), m_idx(index), m_node(dirNode) -{ -} - -StreamIndexedIO::Node::~Node() +StreamIndexedIO::Node::Node(Index* index, DirectoryNode *dirNode) : m_idx(index), m_node(dirNode) { } @@ -1876,6 +1870,10 @@ void StreamIndexedIO::open( StreamFilePtr file, const IndexedIO::EntryIDList &ro StreamIndexedIO::~StreamIndexedIO() { + if ( m_node ) + { + delete m_node; + } // \todo Consider a mechanism for deallocating sub-indexes } From 0ceb10f714e6b513884365a105b07919b6ed0eef Mon Sep 17 00:00:00 2001 From: Lucio Moser Date: Mon, 24 Feb 2014 20:57:48 -0800 Subject: [PATCH 04/11] Fixing bug by checking if pointer is valid. Now all tests passing. --- src/IECore/StreamIndexedIO.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/IECore/StreamIndexedIO.cpp b/src/IECore/StreamIndexedIO.cpp index 06db1a2129..a57b586b63 100644 --- a/src/IECore/StreamIndexedIO.cpp +++ b/src/IECore/StreamIndexedIO.cpp @@ -1157,9 +1157,14 @@ void StreamIndexedIO::Index::read( F &f ) // force sorting all children so that read-only is multi-threaded for (IndexToNodeMap::const_iterator it = m_indexToNodeMap.begin(); it != m_indexToNodeMap.end(); it++ ) { - if ( (*it)->m_nodeType == NodeBase::Directory ) + NodeBase *n = *it; + if ( !n ) { - static_cast< DirectoryNode * >(*it)->sortChildren(); + continue; + } + if ( n->m_nodeType == NodeBase::Directory ) + { + static_cast< DirectoryNode * >(n)->sortChildren(); } } From 52a57b36a4b9e507ec71c4b1b7a8493bce1f4569 Mon Sep 17 00:00:00 2001 From: Lucio Moser Date: Mon, 24 Feb 2014 21:10:29 -0800 Subject: [PATCH 05/11] Adding SmallDataNode class (24 bytes long on 64bit machines) when the data is small enough and avoid the big DataNode struct (40 bytes). --- src/IECore/StreamIndexedIO.cpp | 411 +++++++++++++++++++-------------- 1 file changed, 238 insertions(+), 173 deletions(-) diff --git a/src/IECore/StreamIndexedIO.cpp b/src/IECore/StreamIndexedIO.cpp index a57b586b63..b61acfe9bb 100644 --- a/src/IECore/StreamIndexedIO.cpp +++ b/src/IECore/StreamIndexedIO.cpp @@ -34,7 +34,6 @@ #define __STDC_LIMIT_MACROS #include - #include #include #include @@ -296,6 +295,7 @@ class NodeBase public : typedef enum { + SmallData, Data, Directory } NodeType; @@ -317,24 +317,52 @@ class NodeBase }; -/// Class that represents a Data node -class DataNode : public NodeBase +/// Class that represents small data nodes +class SmallDataNode : public NodeBase { public : - DataNode() : NodeBase(NodeBase::Data) {} + typedef uint16_t Length; + typedef uint32_t Size; + + static const size_t maxArrayLength = UINT16_MAX; + static const size_t maxSize = UINT32_MAX; + + SmallDataNode() : NodeBase(NodeBase::SmallData) {} /// data fields from IndexedIO::Entry - IndexedIO::DataType m_dataType; + // using char instead of enum to compact members in one word + char m_dataType; /// data fields from IndexedIO::Entry - unsigned long m_arrayLength; + Length m_arrayLength; + + /// The size of this node's data chunk within the file + Size m_size; /// The offset in the file to this node's data Imf::Int64 m_offset; +}; + +/// Class that represents Data nodes +class DataNode : public NodeBase +{ + public : + static const size_t maxArrayLength = UINT64_MAX; + static const size_t maxSize = UINT64_MAX; + + DataNode() : NodeBase(NodeBase::Data) {} + + /// data fields from IndexedIO::Entry + IndexedIO::DataType m_dataType; + + /// data fields from IndexedIO::Entry + Imf::Int64 m_arrayLength; /// The size of this node's data chunk within the file Imf::Int64 m_size; + /// The offset in the file to this node's data + Imf::Int64 m_offset; }; /// A directory node within an index @@ -361,7 +389,7 @@ class DirectoryNode : public NodeBase /// A pointer to the parent node in the tree - will be NULL for the root node DirectoryNode* m_parent; - /// Sorted list of node's children (DirectoryNode or DataNode) + /// Sorted list of node's children (DirectoryNode or DataNode/SmallDataNode) typedef std::vector< NodeBase* > ChildMap; ChildMap m_children; @@ -415,10 +443,11 @@ class StreamIndexedIO::Node // Returns the named child node or NULL if not existent. // If loadChildren is true, then it loads the subindex for the child nodes (if applicable). DirectoryNode* child( const IndexedIO::EntryID &name, bool loadChildren ) const; - DataNode* dataChild( const IndexedIO::EntryID &name ) const; + /// returns information about the Data node + inline bool dataChildInfo( const IndexedIO::EntryID &name, size_t &offset, size_t &size ) const; DirectoryNode* addChild( const IndexedIO::EntryID & childName ); - DataNode* addDataChild( const IndexedIO::EntryID & childName ); + void addDataChild( const IndexedIO::EntryID & childName, IndexedIO::DataType dataType, size_t arrayLen, size_t offset, size_t size ); void removeChild( const IndexedIO::EntryID &childName, bool throwException = true ); @@ -446,8 +475,9 @@ class StreamIndexedIO::Index : public RefCounted /// Allocate a new chunk of data of the requested size, returning its offset within the file Imf::Int64 allocate( Imf::Int64 sz ); - /// Deallocate a node's data from the file. - void deallocate( DataNode* n ); + /// Deallocate a Data node's data block from the file. + template< typename D > + void deallocate( D* n ); /// Queries the string cache StringCache &stringCache(); @@ -523,19 +553,19 @@ class StreamIndexedIO::Index : public RefCounted void writeNode( DirectoryNode *n, F &f ); /// Write the data node to a stream - template < typename F > - void writeDataNode( DataNode *n, F &f ); + template < typename F, typename D > + void writeDataNode( D *n, F &f ); template < typename F > void read( F &f ); /// Read method used on previous file format versions up to version 4 - /// Returns a newly created Node or DataNode. + /// Returns a newly created Node. template < typename F > NodeBase *readNodeV4( F &f ); /// Replace the contents of this node with data read from a stream. - /// Returns a newly created Node or DataNode. + /// Returns a newly created Node. template < typename F > NodeBase *readNode( F &f ); @@ -630,18 +660,27 @@ DirectoryNode* StreamIndexedIO::Node::child( const IndexedIO::EntryID &name, boo return n; } -DataNode* StreamIndexedIO::Node::dataChild( const IndexedIO::EntryID &name ) const +bool StreamIndexedIO::Node::dataChildInfo( const IndexedIO::EntryID &name, size_t &offset, size_t &size ) const { - DataNode *n = 0; NodeBase *p = child(name); if ( p ) { if ( p->m_nodeType == NodeBase::Data ) { - n = static_cast< DataNode *>( p ); + DataNode *n = static_cast< DataNode *>( p ); + offset = n->m_offset; + size = n->m_size; + return true; + } + else if ( p->m_nodeType == NodeBase::SmallData ) + { + SmallDataNode *n = static_cast< SmallDataNode *>( p ); + offset = n->m_offset; + size = n->m_size; + return true; } } - return n; + return false; } DirectoryNode* StreamIndexedIO::Node::addChild( const IndexedIO::EntryID &childName ) @@ -671,7 +710,7 @@ DirectoryNode* StreamIndexedIO::Node::addChild( const IndexedIO::EntryID &childN return child; } -DataNode* StreamIndexedIO::Node::addDataChild( const IndexedIO::EntryID &childName ) +void StreamIndexedIO::Node::addDataChild( const IndexedIO::EntryID &childName, IndexedIO::DataType dataType, size_t arrayLen, size_t offset, size_t size ) { if ( m_node->m_subindex ) { @@ -680,24 +719,40 @@ DataNode* StreamIndexedIO::Node::addDataChild( const IndexedIO::EntryID &childNa if ( hasChild(childName) ) { - return 0; + throw IOException( "StreamIndexedIO: Could not insert node '" + childName.value() + "' into index" ); } - DataNode* child = new DataNode; - if ( !child ) - { - throw Exception( "Failed to allocate node!" ); - } - child->m_name = childName; - child->m_dataType = IndexedIO::Invalid; - child->m_arrayLength = 0; m_idx->m_stringCache.add( childName ); - m_node->registerChild( child ); - + if ( arrayLen <= SmallDataNode::maxArrayLength && size <= SmallDataNode::maxSize ) + { + SmallDataNode* child = new SmallDataNode; + if ( !child ) + { + throw Exception( "Failed to allocate node!" ); + } + child->m_name = childName; + child->m_dataType = dataType; + child->m_arrayLength = static_cast(arrayLen); + child->m_offset = offset; + child->m_size = static_cast(size); + m_node->registerChild( child ); + } + else + { + DataNode* child = new DataNode; + if ( !child ) + { + throw Exception( "Failed to allocate node!" ); + } + child->m_name = childName; + child->m_dataType = dataType; + child->m_arrayLength = arrayLen; + child->m_offset = offset; + child->m_size = size; + m_node->registerChild( child ); + } m_idx->m_hasChanged = true; - - return child; } const IndexedIO::EntryID &StreamIndexedIO::Node::name() const @@ -815,6 +870,12 @@ void StreamIndexedIO::Index::recursiveNodeDealloc( NodeBase *n ) delete dn; break; } + case NodeBase::SmallData : + { + SmallDataNode *dn = static_cast< SmallDataNode *>(n); + delete dn; + break; + } default: throw Exception("Unknown node type!"); } @@ -970,7 +1031,7 @@ NodeBase *StreamIndexedIO::Index::readNodeV4( F &f ) n->m_name = *id; n->m_dataType = dataType; - n->m_arrayLength = static_cast( arrayLength ); + n->m_arrayLength = arrayLength; if ( isLink ) // Only version 4 { @@ -1076,13 +1137,30 @@ NodeBase *StreamIndexedIO::Index::readNode( F &f ) readLittleEndian( f,arrayLength ); } - DataNode *n = new DataNode; - n->m_name = m_stringCache.findById( stringId ); - n->m_dataType = dataType; - n->m_arrayLength = static_cast( arrayLength ); - readLittleEndian( f,n->m_offset ); - readLittleEndian( f,n->m_size ); - return n; + Imf::Int64 offset, size; + readLittleEndian( f, offset ); + readLittleEndian( f, size ); + + if ( arrayLength <= SmallDataNode::maxArrayLength && size <= SmallDataNode::maxSize ) + { + SmallDataNode *n = new SmallDataNode; + n->m_name = m_stringCache.findById( stringId ); + n->m_dataType = dataType; + n->m_offset = offset; + n->m_arrayLength = static_cast( arrayLength ); + n->m_size = static_cast( size ); + return n; + } + else + { + DataNode *n = new DataNode; + n->m_name = m_stringCache.findById( stringId ); + n->m_dataType = dataType; + n->m_offset = offset; + n->m_arrayLength = arrayLength; + n->m_size = size; + return n; + } } else if ( entryType == IndexedIO::Directory ) { @@ -1220,8 +1298,8 @@ void StreamIndexedIO::Index::read( F &f ) } } -template < typename F > -void StreamIndexedIO::Index::writeDataNode( DataNode *node, F &f ) +template < typename F, typename D > +void StreamIndexedIO::Index::writeDataNode( D *node, F &f ) { char t = IndexedIO::File; f.write( &t, sizeof(char) ); @@ -1232,13 +1310,13 @@ void StreamIndexedIO::Index::writeDataNode( DataNode *node, F &f ) t = node->m_dataType; f.write( &t, sizeof(char) ); - if ( IndexedIO::Entry::isArray( node->m_dataType ) ) + if ( IndexedIO::Entry::isArray( static_cast(node->m_dataType) ) ) { writeLittleEndian( f, node->m_arrayLength ); } writeLittleEndian(f, node->m_offset); - writeLittleEndian(f, node->m_size); + writeLittleEndian(f, node->m_size); } template < typename F > @@ -1261,15 +1339,26 @@ void StreamIndexedIO::Index::writeNode( DirectoryNode *node, F &f ) for (DirectoryNode::ChildMap::const_iterator it = node->m_children.begin(); it != node->m_children.end(); ++it) { NodeBase *p = *it; - if ( p->m_nodeType == NodeBase::Data ) + switch( p->m_nodeType ) { - DataNode *child = static_cast< DataNode * >(p); - writeDataNode( child, f ); - } - else - { - DirectoryNode *child = static_cast< DirectoryNode * >(p); - writeNode( child, f ); + case NodeBase::Data : + { + DataNode *child = static_cast< DataNode * >(p); + writeDataNode( child, f ); + break; + } + case NodeBase::SmallData : + { + SmallDataNode *child = static_cast< SmallDataNode * >(p); + writeDataNode( child, f ); + break; + } + case NodeBase::Directory : + { + DirectoryNode *child = static_cast< DirectoryNode * >(p); + writeNode( child, f ); + break; + } } } } @@ -1395,10 +1484,11 @@ Imf::Int64 StreamIndexedIO::Index::allocate( Imf::Int64 sz ) return loc; } -void StreamIndexedIO::Index::deallocate( DataNode* n ) +template< typename D > +void StreamIndexedIO::Index::deallocate( D* n ) { assert(n); - assert(n->entryType() == IndexedIO::File); + assert(n->m_nodeType == NodeBase::Data || n->m_nodeType == NodeBase::SmallData ); addFreePage( n->m_offset, n->m_size ); } @@ -1638,16 +1728,26 @@ void StreamIndexedIO::Index::commitNodeToSubIndex( DirectoryNode *n ) for (DirectoryNode::ChildMap::const_iterator it = n->m_children.begin(); it != n->m_children.end(); ++it) { NodeBase *p = *it; - if ( p->m_nodeType == NodeBase::Data ) + switch( p->m_nodeType ) { - DataNode *childNode = static_cast< DataNode *>(p); - writeDataNode( childNode, compressingStream ); - } - else - { - DirectoryNode *childNode = static_cast< DirectoryNode *>(p); - - writeNode( childNode, compressingStream ); + case NodeBase::SmallData: + { + SmallDataNode *childNode = static_cast< SmallDataNode *>(p); + writeDataNode( childNode, compressingStream ); + break; + } + case NodeBase::Data: + { + DataNode *childNode = static_cast< DataNode *>(p); + writeDataNode( childNode, compressingStream ); + break; + } + case NodeBase::Directory: + { + DirectoryNode *childNode = static_cast< DirectoryNode *>(p); + writeNode( childNode, compressingStream ); + break; + } } } @@ -2082,13 +2182,26 @@ IndexedIO::Entry StreamIndexedIO::entry(const IndexedIO::EntryID &name) const throw IOException( "StreamIndexedIO: Entry not found '" + name.value() + "'" ); } - if ( node->m_nodeType == NodeBase::Data ) + switch( node->m_nodeType ) { - DataNode *dn = static_cast< DataNode * >(node); - return IndexedIO::Entry( dn->m_name, IndexedIO::File, dn->m_dataType, dn->m_arrayLength ); - } + case NodeBase::Data: + { + DataNode *dn = static_cast< DataNode * >(node); + return IndexedIO::Entry( dn->m_name, IndexedIO::File, static_cast(dn->m_dataType), dn->m_arrayLength ); + } + + case NodeBase::SmallData: + { + SmallDataNode *dn = static_cast< SmallDataNode * >(node); + return IndexedIO::Entry( dn->m_name, IndexedIO::File, static_cast(dn->m_dataType), dn->m_arrayLength ); + } - return IndexedIO::Entry( node->m_name, IndexedIO::Directory, IndexedIO::Invalid, 0 ); + case NodeBase::Directory: + return IndexedIO::Entry( node->m_name, IndexedIO::Directory, IndexedIO::Invalid, 0 ); + + default: + throw Exception("Invalid node type!"); + } } IndexedIOPtr StreamIndexedIO::parentDirectory() @@ -2165,12 +2278,6 @@ void StreamIndexedIO::write(const IndexedIO::EntryID &name, const InternedString writable(name); remove(name, false); - DataNode* node = m_node->addDataChild( name ); - if (!node) - { - throw IOException( "StreamIndexedIO: Could not insert node '" + name.value() + "' into index" ); - } - Imf::Int64 *ids = new Imf::Int64[arrayLength]; const Imf::Int64 *constIds = ids; unsigned long size = IndexedIO::DataSizeTraits::size(constIds, arrayLength); @@ -2190,10 +2297,9 @@ void StreamIndexedIO::write(const IndexedIO::EntryID &name, const InternedString IndexedIO::DataFlattenTraits::flatten(constIds, arrayLength, data); - node->m_dataType = dataType; - node->m_arrayLength = arrayLength; - node->m_offset = index->writeUniqueData( data, size ); - node->m_size = size; + size_t offset = index->writeUniqueData( data, size ); + + m_node->addDataChild( name, dataType, arrayLength, offset, size ); delete [] ids; } @@ -2203,27 +2309,27 @@ void StreamIndexedIO::read(const IndexedIO::EntryID &name, InternedString *&x, u assert( m_node ); readable(name); - DataNode* node = m_node->dataChild( name ); + Imf::Int64 dataOffset, dataSize; - if (!node) + if ( !m_node->dataChildInfo( name, dataOffset, dataSize ) ) { throw IOException( "StreamIndexedIO: Entry not found '" + name.value() + "'" ); } Imf::Int64 *ids = new Imf::Int64[arrayLength]; - Imf::Int64 size = node->m_size; + StreamIndexedIO::StreamFile &f = streamFile(); { StreamFile::MutexLock lock( f.mutex() ); - f.seekg( node->m_offset, std::ios::beg ); + f.seekg( dataOffset, std::ios::beg ); #ifdef IE_CORE_LITTLE_ENDIAN // raw read - f.read( (char*)ids, size ); + f.read( (char*)ids, dataSize ); } #else - char *data = f.ioBuffer(size); - f.read( data, size ); + char *data = f.ioBuffer(dataSize); + f.read( data, dataSize ); } IndexedIO::DataFlattenTraits::unflatten( data, ids, arrayLength ); #endif @@ -2247,26 +2353,16 @@ void StreamIndexedIO::write(const IndexedIO::EntryID &name, const T *x, unsigned writable(name); remove(name, false); - DataNode* node = m_node->addDataChild( name ); - if (node) - { - unsigned long size = IndexedIO::DataSizeTraits::size(x, arrayLength); - IndexedIO::DataType dataType = IndexedIO::DataTypeTraits::type(); + unsigned long size = IndexedIO::DataSizeTraits::size(x, arrayLength); + IndexedIO::DataType dataType = IndexedIO::DataTypeTraits::type(); - char *data = streamFile().ioBuffer(size); - assert(data); - IndexedIO::DataFlattenTraits::flatten(x, arrayLength, data); + char *data = streamFile().ioBuffer(size); + assert(data); + IndexedIO::DataFlattenTraits::flatten(x, arrayLength, data); - node->m_dataType = dataType; - node->m_arrayLength = arrayLength; - node->m_offset = m_node->m_idx->writeUniqueData( data, size ); - node->m_size = size; - } + Imf::Int64 offset = m_node->m_idx->writeUniqueData( data, size ); - else - { - throw IOException( "StreamIndexedIO: Could not insert node '" + name.value() + "' into index" ); - } + m_node->addDataChild( name, dataType, arrayLength, offset, size ); } template @@ -2275,21 +2371,12 @@ void StreamIndexedIO::rawWrite(const IndexedIO::EntryID &name, const T *x, unsig writable(name); remove(name, false); - DataNode* node = m_node->addDataChild( name ); - if (node) - { - unsigned long size = IndexedIO::DataSizeTraits::size(x, arrayLength); - IndexedIO::DataType dataType = IndexedIO::DataTypeTraits::type(); + unsigned long size = IndexedIO::DataSizeTraits::size(x, arrayLength); + IndexedIO::DataType dataType = IndexedIO::DataTypeTraits::type(); - node->m_dataType = dataType; - node->m_arrayLength = arrayLength; - node->m_offset = m_node->m_idx->writeUniqueData( (char*)x, size ); - node->m_size = size; - } - else - { - throw IOException( "StreamIndexedIO: Could not insert node '" + name.value() + "' into index" ); - } + Imf::Int64 offset = m_node->m_idx->writeUniqueData( (char*)x, size ); + + m_node->addDataChild( name, dataType, arrayLength, offset, size ); } template @@ -2298,25 +2385,16 @@ void StreamIndexedIO::write(const IndexedIO::EntryID &name, const T &x) writable(name); remove(name, false); - DataNode* node = m_node->addDataChild( name ); - if (node) - { - unsigned long size = IndexedIO::DataSizeTraits::size(x); - IndexedIO::DataType dataType = IndexedIO::DataTypeTraits::type(); + unsigned long size = IndexedIO::DataSizeTraits::size(x); + IndexedIO::DataType dataType = IndexedIO::DataTypeTraits::type(); - char *data = streamFile().ioBuffer(size); - assert(data); - IndexedIO::DataFlattenTraits::flatten(x, data); + char *data = streamFile().ioBuffer(size); + assert(data); + IndexedIO::DataFlattenTraits::flatten(x, data); - node->m_dataType = dataType; - node->m_arrayLength = 0; - node->m_offset = m_node->m_idx->writeUniqueData( data, size ); - node->m_size = size; - } - else - { - throw IOException( "StreamIndexedIO: Could not insert node '" + name.value() + "' into index" ); - } + Imf::Int64 offset = m_node->m_idx->writeUniqueData( data, size ); + + m_node->addDataChild( name, dataType, 0, offset, size ); } template @@ -2325,21 +2403,12 @@ void StreamIndexedIO::rawWrite(const IndexedIO::EntryID &name, const T &x) writable(name); remove(name, false); - DataNode* node = m_node->addDataChild( name ); - if (node) - { - unsigned long size = IndexedIO::DataSizeTraits::size(x); - IndexedIO::DataType dataType = IndexedIO::DataTypeTraits::type(); + unsigned long size = IndexedIO::DataSizeTraits::size(x); + IndexedIO::DataType dataType = IndexedIO::DataTypeTraits::type(); - node->m_dataType = dataType; - node->m_arrayLength = 0; - node->m_offset = m_node->m_idx->writeUniqueData( (char*)&x, size ); - node->m_size = size; - } - else - { - throw IOException( "StreamIndexedIO: Could not insert node '" + name.value() + "' into index" ); - } + Imf::Int64 offset = m_node->m_idx->writeUniqueData( (char*)&x, size ); + + m_node->addDataChild( name, dataType, 0, offset, size ); } template @@ -2348,20 +2417,19 @@ void StreamIndexedIO::read(const IndexedIO::EntryID &name, T *&x, unsigned long assert( m_node ); readable(name); - DataNode* node = m_node->dataChild( name ); + Imf::Int64 dataOffset, dataSize; - if (!node) + if ( !m_node->dataChildInfo( name, dataOffset, dataSize ) ) { throw IOException( "StreamIndexedIO: Entry not found '" + name.value() + "'" ); } StreamIndexedIO::StreamFile &f = streamFile(); - Imf::Int64 size = node->m_size; { StreamFile::MutexLock lock( f.mutex() ); - char *data = f.ioBuffer(size); - f.seekg( node->m_offset, std::ios::beg ); - f.read( data, size ); + char *data = f.ioBuffer(dataSize); + f.seekg( dataOffset, std::ios::beg ); + f.read( data, dataSize ); IndexedIO::DataFlattenTraits::unflatten( data, x, arrayLength ); } } @@ -2372,14 +2440,13 @@ void StreamIndexedIO::rawRead(const IndexedIO::EntryID &name, T *&x, unsigned lo assert( m_node ); readable(name); - DataNode* node = m_node->dataChild( name ); + Imf::Int64 dataOffset, dataSize; - if (!node) + if ( !m_node->dataChildInfo( name, dataOffset, dataSize ) ) { - throw IOException( "StreamIndexedIO: Entry not found '" + name.value() + "'" ); + throw IOException( "StreamIndexedIO: Data entry not found '" + name.value() + "'" ); } - Imf::Int64 size = node->m_size; if (!x) { x = new T[arrayLength]; @@ -2388,8 +2455,8 @@ void StreamIndexedIO::rawRead(const IndexedIO::EntryID &name, T *&x, unsigned lo StreamIndexedIO::StreamFile &f = streamFile(); { StreamFile::MutexLock lock( f.mutex() ); - f.seekg( node->m_offset, std::ios::beg ); - f.read( (char*)x, size ); + f.seekg( dataOffset, std::ios::beg ); + f.read( (char*)x, dataSize ); } } @@ -2399,20 +2466,19 @@ void StreamIndexedIO::read(const IndexedIO::EntryID &name, T &x) const assert( m_node ); readable(name); - DataNode* node = m_node->dataChild( name ); + Imf::Int64 dataOffset, dataSize; - if (!node) + if ( !m_node->dataChildInfo( name, dataOffset, dataSize ) ) { - throw IOException( "StreamIndexedIO: Entry not found '" + name.value() + "'" ); + throw IOException( "StreamIndexedIO: Data entry not found '" + name.value() + "'" ); } - Imf::Int64 size = node->m_size; StreamIndexedIO::StreamFile &f = streamFile(); { StreamFile::MutexLock lock( f.mutex() ); - char *data = f.ioBuffer(size); - f.seekg( node->m_offset, std::ios::beg ); - f.read( data, size ); + char *data = f.ioBuffer(dataSize); + f.seekg( dataOffset, std::ios::beg ); + f.read( data, dataSize ); IndexedIO::DataFlattenTraits::unflatten( data, x ); } } @@ -2423,19 +2489,18 @@ void StreamIndexedIO::rawRead(const IndexedIO::EntryID &name, T &x) const assert( m_node ); readable(name); - DataNode* node = m_node->dataChild( name ); + Imf::Int64 dataOffset, dataSize; - if (!node) + if ( !m_node->dataChildInfo( name, dataOffset, dataSize ) ) { - throw IOException( "StreamIndexedIO: Entry not found '" + name.value() + "'" ); + throw IOException( "StreamIndexedIO: Data entry not found '" + name.value() + "'" ); } - Imf::Int64 size = node->m_size; StreamIndexedIO::StreamFile &f = streamFile(); { StreamFile::MutexLock lock( f.mutex() ); - f.seekg( node->m_offset, std::ios::beg ); - f.read( (char*)&x, size ); + f.seekg( dataOffset, std::ios::beg ); + f.read( (char*)&x, dataSize ); } } From e955831ef5169d1fb94197360875bf34d56358fb Mon Sep 17 00:00:00 2001 From: Lucio Moser Date: Mon, 24 Feb 2014 22:47:40 -0800 Subject: [PATCH 06/11] Updating comment. --- include/IECore/StreamIndexedIO.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/IECore/StreamIndexedIO.h b/include/IECore/StreamIndexedIO.h index 19a5507ff7..1d16512be9 100644 --- a/include/IECore/StreamIndexedIO.h +++ b/include/IECore/StreamIndexedIO.h @@ -257,7 +257,7 @@ class StreamIndexedIO : public IndexedIO private : - // \todo Consider exposing Node to avoid too much memory fragmentation and save one raw pointer. + // \todo Either rename Node to MemberData or add it's member here and save one raw pointer. Node *m_node; void setRoot( const IndexedIO::EntryIDList &root ); From 9303ae5202a37b1d19683f8fba0761125e1104bb Mon Sep 17 00:00:00 2001 From: Lucio Moser Date: Mon, 24 Feb 2014 22:48:26 -0800 Subject: [PATCH 07/11] Adding a compact Node class for representing non-expanded subIndexes: SubIndexNode (24 bytes). This helps on the memory requirements for opening a file with a large amount of stored Objects. In particular, large LinkedScenes. --- src/IECore/StreamIndexedIO.cpp | 309 +++++++++++++++++++-------------- 1 file changed, 174 insertions(+), 135 deletions(-) diff --git a/src/IECore/StreamIndexedIO.cpp b/src/IECore/StreamIndexedIO.cpp index b61acfe9bb..5550662182 100644 --- a/src/IECore/StreamIndexedIO.cpp +++ b/src/IECore/StreamIndexedIO.cpp @@ -49,6 +49,7 @@ #include "boost/iostreams/filtering_stream.hpp" #include "boost/iostreams/stream.hpp" #include "boost/iostreams/filter/gzip.hpp" +#include "tbb/spin_rw_mutex.h" #include "IECore/ByteOrder.h" #include "IECore/MemoryStream.h" @@ -297,7 +298,8 @@ class NodeBase typedef enum { SmallData, Data, - Directory + Directory, + SubIndex } NodeType; NodeBase( IndexedIO::EntryID name ) : m_name(name) {} @@ -365,6 +367,17 @@ class DataNode : public NodeBase Imf::Int64 m_offset; }; +/// A compressed subindex node +class SubIndexNode : public NodeBase +{ + public : + SubIndexNode() : NodeBase(NodeBase::SubIndex) {} + + /// The offset in the file to this node's subindex block if m_subindex is not NoSubIndex. + Imf::Int64 m_offset; + +}; + /// A directory node within an index /// It also represents subindex directory nodes by setting m_subindex at the root and all it's child nodes to true. class DirectoryNode : public NodeBase @@ -440,9 +453,8 @@ class StreamIndexedIO::Node /// Returns a Node or DataNode or NULL if not existent. NodeBase* child( const IndexedIO::EntryID &name ) const; - // Returns the named child node or NULL if not existent. - // If loadChildren is true, then it loads the subindex for the child nodes (if applicable). - DirectoryNode* child( const IndexedIO::EntryID &name, bool loadChildren ) const; + // Returns the named child directory node or NULL if not existent. Loads the subindex for the child nodes (if applicable). + DirectoryNode* directoryChild( const IndexedIO::EntryID &name ) const; /// returns information about the Data node inline bool dataChildInfo( const IndexedIO::EntryID &name, size_t &offset, size_t &size ) const; @@ -497,6 +509,10 @@ class StreamIndexedIO::Index : public RefCounted /// read the subindex that contains the children of the given node void readNodeFromSubIndex( DirectoryNode *n ); + /// controls thread-safe access to the Node hierarchy + typedef tbb::spin_rw_mutex Mutex; + Mutex m_mutex; + protected: DirectoryNode *m_root; @@ -552,10 +568,18 @@ class StreamIndexedIO::Index : public RefCounted template < typename F > void writeNode( DirectoryNode *n, F &f ); + /// Write the subindex node to a stream + template < typename F > + void writeNode( SubIndexNode *n, F &f ); + /// Write the data node to a stream template < typename F, typename D > void writeDataNode( D *n, F &f ); + /// Serialize all the node's children to a stream + template < typename F > + void writeNodeChildren( DirectoryNode *n, F &f ); + template < typename F > void read( F &f ); @@ -569,8 +593,6 @@ class StreamIndexedIO::Index : public RefCounted template < typename F > NodeBase *readNode( F &f ); - void recursiveSetSubIndex( DirectoryNode *n ); - static void recursiveNodeDealloc( NodeBase *n ); }; @@ -641,27 +663,62 @@ NodeBase* StreamIndexedIO::Node::child( const IndexedIO::EntryID &name ) const return 0; } -DirectoryNode* StreamIndexedIO::Node::child( const IndexedIO::EntryID &name, bool loadChildren ) const +DirectoryNode* StreamIndexedIO::Node::directoryChild( const IndexedIO::EntryID &name ) const { - DirectoryNode *n = 0; - NodeBase *p = child(name); - if ( p ) + Index::Mutex::scoped_lock lock( m_idx->m_mutex, false ); // read-only lock + + DirectoryNode::ChildMap::iterator it; + if ( m_node->findChild( name, it ) ) { - if ( p->m_nodeType == NodeBase::Directory ) + if ( (*it)->m_nodeType == NodeBase::Directory ) { - n = static_cast< DirectoryNode *>( p ); + DirectoryNode *dir = static_cast< DirectoryNode *>( (*it) ); + + if ( dir->m_subindex == DirectoryNode::SavedSubIndex ) + { + // this can occur when the user flushed a directory and right after tries to access it. + m_idx->readNodeFromSubIndex( dir ); + } + return dir; + } + else if ( (*it)->m_nodeType == NodeBase::SubIndex ) + { + SubIndexNode *subIndex = static_cast< SubIndexNode *>( (*it) ); - if ( loadChildren && n->m_subindex ) + DirectoryNode *newDir = new DirectoryNode; + newDir->m_name = subIndex->m_name; + newDir->m_offset = subIndex->m_offset; + newDir->m_parent = m_node; + m_idx->readNodeFromSubIndex( newDir ); + { - m_idx->readNodeFromSubIndex( n ); + // now that we loaded the whole thing, lock our Index for writing and replace the Node pointer + lock.upgrade_to_writer(); + + // there's a chance that someone else already replaced the pointer... + if ( (*it)->m_nodeType == NodeBase::Directory ) + { + Index::recursiveNodeDealloc( newDir ); + return static_cast< DirectoryNode *>(*it); + } + + // replace SubIndex by Directory node. + (*it) = newDir; } + + // and now we are ok to delete the SubIndexNode.. + delete subIndex; + + return newDir; } } - return n; + return 0; } bool StreamIndexedIO::Node::dataChildInfo( const IndexedIO::EntryID &name, size_t &offset, size_t &size ) const { + Index::Mutex::scoped_lock lock( m_idx->m_mutex, false ); // read-only lock + NodeBase *p = child(name); if ( p ) { @@ -876,6 +933,12 @@ void StreamIndexedIO::Index::recursiveNodeDealloc( NodeBase *n ) delete dn; break; } + case NodeBase::SubIndex : + { + SubIndexNode *dn = static_cast< SubIndexNode *>(n); + delete dn; + break; + } default: throw Exception("Unknown node type!"); } @@ -1062,6 +1125,7 @@ NodeBase *StreamIndexedIO::Index::readNodeV4( F &f ) { DirectoryNode *n = new DirectoryNode(); n->m_name = *id; + if ( m_version < 2 ) { Imf::Int64 size; @@ -1110,23 +1174,15 @@ NodeBase *StreamIndexedIO::Index::readNodeV4( F &f ) template < typename F > NodeBase *StreamIndexedIO::Index::readNode( F &f ) { - char t; - f.read( &t, sizeof(char) ); - - IndexedIO::EntryType entryType = (IndexedIO::EntryType)t; - DirectoryNode::SubIndexMode subindex = DirectoryNode::NoSubIndex; - - if ( t == SUBINDEX_DIR ) - { - entryType = IndexedIO::Directory; - subindex = DirectoryNode::SavedSubIndex; - } + char entryType; + f.read( &entryType, sizeof(char) ); Imf::Int64 stringId; readLittleEndian(f,stringId); if ( entryType == IndexedIO::File ) { + char t; IndexedIO::DataType dataType = IndexedIO::Invalid; Imf::Int64 arrayLength = 0; f.read( &t, sizeof(char) ); @@ -1166,27 +1222,25 @@ NodeBase *StreamIndexedIO::Index::readNode( F &f ) { DirectoryNode *n = new DirectoryNode(); n->m_name = m_stringCache.findById( stringId ); - n->m_subindex = subindex; - - if ( subindex ) - { - readLittleEndian( f,n->m_offset ); - } - else - { - n->m_offset = 0; + n->m_subindex = DirectoryNode::NoSubIndex; - uint32_t nodeCount = 0; - readLittleEndian( f, nodeCount ); + uint32_t nodeCount = 0; + readLittleEndian( f, nodeCount ); - for ( uint32_t c = 0; c < nodeCount; c++ ) - { - NodeBase *child = readNode( f ); - n->registerChild( child ); - } - // force sorting all children so that read-only is multi-threaded - n->sortChildren(); + for ( uint32_t c = 0; c < nodeCount; c++ ) + { + NodeBase *child = readNode( f ); + n->registerChild( child ); } + // force sorting all children so that read-only is multi-threaded + n->sortChildren(); + return n; + } + else if ( entryType == SUBINDEX_DIR ) + { + SubIndexNode *n = new SubIndexNode(); + n->m_name = m_stringCache.findById( stringId ); + readLittleEndian( f,n->m_offset ); return n; } else @@ -1319,6 +1373,59 @@ void StreamIndexedIO::Index::writeDataNode( D *node, F &f ) writeLittleEndian(f, node->m_size); } +template < typename F > +void StreamIndexedIO::Index::writeNode( SubIndexNode *node, F &f ) +{ + char t = SUBINDEX_DIR; + f.write( &t, sizeof(char) ); + + Imf::Int64 id = m_stringCache.find( node->m_name ); + writeLittleEndian( f, id ); + writeLittleEndian(f, node->m_offset); +} + +template < typename F > +void StreamIndexedIO::Index::writeNodeChildren( DirectoryNode *n, F &f ) +{ + uint32_t nodeCount = n->m_children.size(); + + writeLittleEndian( f, nodeCount ); + + for (DirectoryNode::ChildMap::const_iterator it = n->m_children.begin(); it != n->m_children.end(); ++it) + { + NodeBase *p = *it; + switch( p->m_nodeType ) + { + case NodeBase::Data : + { + DataNode *childNode = static_cast< DataNode *>(p); + writeDataNode( childNode, f ); + break; + } + case NodeBase::SmallData : + { + SmallDataNode *childNode = static_cast< SmallDataNode * >(p); + writeDataNode( childNode, f ); + break; + } + case NodeBase::Directory : + { + DirectoryNode *childNode = static_cast< DirectoryNode *>(p); + writeNode( childNode, f ); + break; + } + case NodeBase::SubIndex : + { + SubIndexNode *childNode = static_cast< SubIndexNode *>(p); + writeNode( childNode, f ); + break; + } + default: + throw Exception( "Invalid node type!" ); + } + } +} + template < typename F > void StreamIndexedIO::Index::writeNode( DirectoryNode *node, F &f ) { @@ -1334,33 +1441,7 @@ void StreamIndexedIO::Index::writeNode( DirectoryNode *node, F &f ) } else { - uint32_t nodeCount = node->m_children.size(); - writeLittleEndian(f, nodeCount); - for (DirectoryNode::ChildMap::const_iterator it = node->m_children.begin(); it != node->m_children.end(); ++it) - { - NodeBase *p = *it; - switch( p->m_nodeType ) - { - case NodeBase::Data : - { - DataNode *child = static_cast< DataNode * >(p); - writeDataNode( child, f ); - break; - } - case NodeBase::SmallData : - { - SmallDataNode *child = static_cast< SmallDataNode * >(p); - writeDataNode( child, f ); - break; - } - case NodeBase::Directory : - { - DirectoryNode *child = static_cast< DirectoryNode * >(p); - writeNode( child, f ); - break; - } - } - } + writeNodeChildren( node, f ); } } @@ -1688,24 +1769,6 @@ void StreamIndexedIO::Index::deallocateWalk( NodeBase* n ) } -void StreamIndexedIO::Index::recursiveSetSubIndex( DirectoryNode *n ) -{ - n->m_subindex = DirectoryNode::SavedSubIndex; - - for (DirectoryNode::ChildMap::const_iterator it = n->m_children.begin(); it != n->m_children.end(); ++it) - { - DirectoryNode *childNode = static_cast(*it); - - if ( childNode->m_nodeType != NodeBase::Directory ) - continue; - - if (childNode && childNode->m_subindex == DirectoryNode::NoSubIndex ) - { - recursiveSetSubIndex( childNode ); - } - } -} - void StreamIndexedIO::Index::commitNodeToSubIndex( DirectoryNode *n ) { if (!n) @@ -1721,35 +1784,7 @@ void StreamIndexedIO::Index::commitNodeToSubIndex( DirectoryNode *n ) compressingStream.push( sink ); assert( compressingStream.is_complete() ); - uint32_t nodeCount = n->m_children.size(); - - writeLittleEndian( compressingStream, nodeCount ); - - for (DirectoryNode::ChildMap::const_iterator it = n->m_children.begin(); it != n->m_children.end(); ++it) - { - NodeBase *p = *it; - switch( p->m_nodeType ) - { - case NodeBase::SmallData: - { - SmallDataNode *childNode = static_cast< SmallDataNode *>(p); - writeDataNode( childNode, compressingStream ); - break; - } - case NodeBase::Data: - { - DataNode *childNode = static_cast< DataNode *>(p); - writeDataNode( childNode, compressingStream ); - break; - } - case NodeBase::Directory: - { - DirectoryNode *childNode = static_cast< DirectoryNode *>(p); - writeNode( childNode, compressingStream ); - break; - } - } - } + writeNodeChildren( n, compressingStream ); compressingStream.pop(); compressingStream.pop(); @@ -1761,8 +1796,14 @@ void StreamIndexedIO::Index::commitNodeToSubIndex( DirectoryNode *n ) n->m_offset = writeUniqueData( data, subindexSize, true ); - // set all child nodes as committed to a subindex (this also makes them read-only) - recursiveSetSubIndex( n ); + // mark this node as a saved in a subindex + n->m_subindex = DirectoryNode::SavedSubIndex; + + // dealloc all the child nodes + for (DirectoryNode::ChildMap::const_iterator it = n->m_children.begin(); it != n->m_children.end(); ++it) + { + recursiveNodeDealloc( *it ); + } // remove all children from this node ( freeing some memory if there's no external references) n->m_children.clear(); @@ -1771,11 +1812,6 @@ void StreamIndexedIO::Index::commitNodeToSubIndex( DirectoryNode *n ) void StreamIndexedIO::Index::readNodeFromSubIndex( DirectoryNode *n ) { - if ( n->m_subindex == DirectoryNode::NoSubIndex ) - { - return; - } - /// guarantees thread safe access to the file and also to the m_subindex variable StreamFile::MutexLock lock( m_stream->mutex() ); @@ -1783,7 +1819,7 @@ void StreamIndexedIO::Index::readNodeFromSubIndex( DirectoryNode *n ) { return; } - + m_stream->seekg( n->m_offset, std::ios::beg ); uint32_t subindexSize = 0; @@ -1987,7 +2023,7 @@ void StreamIndexedIO::setRoot( const IndexedIO::EntryIDList &root ) IndexedIO::EntryIDList::const_iterator t = root.begin(); for ( ; t != root.end(); t++ ) { - DirectoryNode* childNode = m_node->child( *t, true ); + DirectoryNode* childNode = m_node->directoryChild( *t ); if ( !childNode ) { break; @@ -2071,7 +2107,7 @@ bool StreamIndexedIO::hasEntry( const IndexedIO::EntryID &name ) const IndexedIOPtr StreamIndexedIO::subdirectory( const IndexedIO::EntryID &name, IndexedIO::MissingBehaviour missingBehaviour ) { assert( m_node ); - DirectoryNode *childNode = m_node->child( name, true ); + DirectoryNode *childNode = m_node->directoryChild( name ); if ( !childNode ) { if ( missingBehaviour == IndexedIO::CreateIfMissing ) @@ -2100,7 +2136,7 @@ ConstIndexedIOPtr StreamIndexedIO::subdirectory( const IndexedIO::EntryID &name, { readable(name); assert( m_node ); - DirectoryNode *childNode = m_node->child( name, true ); + DirectoryNode *childNode = m_node->directoryChild( name ); if ( !childNode ) { if ( missingBehaviour == IndexedIO::NullIfMissing ) @@ -2175,6 +2211,8 @@ IndexedIO::Entry StreamIndexedIO::entry(const IndexedIO::EntryID &name) const assert( m_node ); readable(name); + Index::Mutex::scoped_lock lock( m_node->m_idx->m_mutex, false ); // read-only lock + NodeBase* node = m_node->child( name ); if (!node) @@ -2197,6 +2235,7 @@ IndexedIO::Entry StreamIndexedIO::entry(const IndexedIO::EntryID &name) const } case NodeBase::Directory: + case NodeBase::SubIndex: return IndexedIO::Entry( node->m_name, IndexedIO::Directory, IndexedIO::Invalid, 0 ); default: @@ -2237,7 +2276,7 @@ IndexedIOPtr StreamIndexedIO::directory( const IndexedIO::EntryIDList &path, Ind { const IndexedIO::EntryID &name = *pIt; - DirectoryNode* childNode = newNode->child( name, true ); + DirectoryNode* childNode = newNode->directoryChild( name ); if ( !childNode ) { if ( missingBehaviour == IndexedIO::CreateIfMissing ) @@ -2309,7 +2348,7 @@ void StreamIndexedIO::read(const IndexedIO::EntryID &name, InternedString *&x, u assert( m_node ); readable(name); - Imf::Int64 dataOffset, dataSize; + Imf::Int64 dataOffset(0), dataSize(0); if ( !m_node->dataChildInfo( name, dataOffset, dataSize ) ) { @@ -2417,7 +2456,7 @@ void StreamIndexedIO::read(const IndexedIO::EntryID &name, T *&x, unsigned long assert( m_node ); readable(name); - Imf::Int64 dataOffset, dataSize; + Imf::Int64 dataOffset(0), dataSize(0); if ( !m_node->dataChildInfo( name, dataOffset, dataSize ) ) { @@ -2440,7 +2479,7 @@ void StreamIndexedIO::rawRead(const IndexedIO::EntryID &name, T *&x, unsigned lo assert( m_node ); readable(name); - Imf::Int64 dataOffset, dataSize; + Imf::Int64 dataOffset(0), dataSize(0); if ( !m_node->dataChildInfo( name, dataOffset, dataSize ) ) { @@ -2466,7 +2505,7 @@ void StreamIndexedIO::read(const IndexedIO::EntryID &name, T &x) const assert( m_node ); readable(name); - Imf::Int64 dataOffset, dataSize; + Imf::Int64 dataOffset(0), dataSize(0); if ( !m_node->dataChildInfo( name, dataOffset, dataSize ) ) { @@ -2489,7 +2528,7 @@ void StreamIndexedIO::rawRead(const IndexedIO::EntryID &name, T &x) const assert( m_node ); readable(name); - Imf::Int64 dataOffset, dataSize; + Imf::Int64 dataOffset(0), dataSize(0); if ( !m_node->dataChildInfo( name, dataOffset, dataSize ) ) { From d6edca6a82292fe9758821e7118ba0cf2c79175f Mon Sep 17 00:00:00 2001 From: Lucio Moser Date: Tue, 25 Feb 2014 16:47:41 -0800 Subject: [PATCH 08/11] Bug fix: considering the new SubIndexNode a IndexedIO::Directory node for the purposes of filtering child entries by type. --- src/IECore/StreamIndexedIO.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/IECore/StreamIndexedIO.cpp b/src/IECore/StreamIndexedIO.cpp index 5550662182..6e37286b6e 100644 --- a/src/IECore/StreamIndexedIO.cpp +++ b/src/IECore/StreamIndexedIO.cpp @@ -837,7 +837,7 @@ void StreamIndexedIO::Node::childNames( IndexedIO::EntryIDList &names, IndexedIO for ( DirectoryNode::ChildMap::const_iterator cit = m_node->m_children.begin(); cit != m_node->m_children.end(); cit++ ) { NodeBase *cc = *cit; - bool childIsDirectory = ( cc->m_nodeType == NodeBase::Directory ); + bool childIsDirectory = ( cc->m_nodeType == NodeBase::Directory || cc->m_nodeType == NodeBase::SubIndex ); if ( typeIsDirectory == childIsDirectory ) { names.push_back( (*cit)->m_name ); @@ -1805,7 +1805,7 @@ void StreamIndexedIO::Index::commitNodeToSubIndex( DirectoryNode *n ) recursiveNodeDealloc( *it ); } - // remove all children from this node ( freeing some memory if there's no external references) + // remove all children from this node n->m_children.clear(); } } From 9b3f82a524ff0d9fb1bd522192b98014142b10ba Mon Sep 17 00:00:00 2001 From: Lucio Moser Date: Thu, 27 Feb 2014 15:58:42 -0800 Subject: [PATCH 09/11] Being more frugal on the usage of the mutex that controls access to the hierarchy of Nodes. Added a boolean in each Directory node that will be true only if there's a child that is SubIndex and only uses mutex on these locations. Also, avoids keeping the mutex locked while reading the subindex or during deallocation of duplicated Directories. --- src/IECore/StreamIndexedIO.cpp | 87 ++++++++++++++++++++-------------- 1 file changed, 51 insertions(+), 36 deletions(-) diff --git a/src/IECore/StreamIndexedIO.cpp b/src/IECore/StreamIndexedIO.cpp index 6e37286b6e..f1f001bcad 100644 --- a/src/IECore/StreamIndexedIO.cpp +++ b/src/IECore/StreamIndexedIO.cpp @@ -391,10 +391,11 @@ class DirectoryNode : public NodeBase LoadedSubIndex, }; - DirectoryNode() : NodeBase(NodeBase::Directory), m_subindex(NoSubIndex), m_sortedChildren(false), m_offset(0), m_parent(0) {} + DirectoryNode() : NodeBase(NodeBase::Directory), m_subindex(NoSubIndex), m_sortedChildren(false), m_subindexChildren(false), m_offset(0), m_parent(0) {} char m_subindex; // using char instead of enum to compact members in one word bool m_sortedChildren; // same as above + bool m_subindexChildren; // true if one or more children are subindex. Helps avoiding the mutex... /// The offset in the file to this node's subindex block if m_subindex is not NoSubIndex. Imf::Int64 m_offset; @@ -451,8 +452,6 @@ class StreamIndexedIO::Node bool hasChild( const IndexedIO::EntryID &name ) const; - /// Returns a Node or DataNode or NULL if not existent. - NodeBase* child( const IndexedIO::EntryID &name ) const; // Returns the named child directory node or NULL if not existent. Loads the subindex for the child nodes (if applicable). DirectoryNode* directoryChild( const IndexedIO::EntryID &name ) const; /// returns information about the Data node @@ -624,6 +623,10 @@ void DirectoryNode::registerChild( NodeBase* c ) } childNode->m_parent = this; } + else if ( c->m_nodeType == NodeBase::SubIndex ) + { + m_subindexChildren = true; + } m_children.push_back( c ); m_sortedChildren = false; } @@ -653,29 +656,28 @@ bool StreamIndexedIO::Node::hasChild( const IndexedIO::EntryID &name ) const return m_node->findChild( name, cit ); } -NodeBase* StreamIndexedIO::Node::child( const IndexedIO::EntryID &name ) const -{ - DirectoryNode::ChildMap::const_iterator cit; - if ( m_node->findChild( name, cit ) ) - { - return *cit; - } - return 0; -} - DirectoryNode* StreamIndexedIO::Node::directoryChild( const IndexedIO::EntryID &name ) const { - Index::Mutex::scoped_lock lock( m_idx->m_mutex, false ); // read-only lock - DirectoryNode::ChildMap::iterator it; if ( m_node->findChild( name, it ) ) { + Index::Mutex::scoped_lock lock; + if ( m_node->m_subindexChildren ) + { + lock.acquire( m_idx->m_mutex, false ); // read-only lock + } + if ( (*it)->m_nodeType == NodeBase::Directory ) { DirectoryNode *dir = static_cast< DirectoryNode *>( (*it) ); if ( dir->m_subindex == DirectoryNode::SavedSubIndex ) { + if ( m_node->m_subindexChildren ) + { + lock.release(); /// we will not change the children dictionary, so we release the lock! + } + // this can occur when the user flushed a directory and right after tries to access it. m_idx->readNodeFromSubIndex( dir ); } @@ -689,22 +691,24 @@ DirectoryNode* StreamIndexedIO::Node::directoryChild( const IndexedIO::EntryID & newDir->m_name = subIndex->m_name; newDir->m_offset = subIndex->m_offset; newDir->m_parent = m_node; + + lock.release(); /// we release the lock while loading data.. + m_idx->readNodeFromSubIndex( newDir ); - { - // now that we loaded the whole thing, lock our Index for writing and replace the Node pointer - lock.upgrade_to_writer(); + // now that we loaded the whole thing, lock our Index for writing + lock.acquire( m_idx->m_mutex, true ); - // there's a chance that someone else already replaced the pointer... - if ( (*it)->m_nodeType == NodeBase::Directory ) - { - Index::recursiveNodeDealloc( newDir ); - return static_cast< DirectoryNode *>(*it); - } - - // replace SubIndex by Directory node. - (*it) = newDir; + // there's a chance that someone else already replaced the pointer... + if ( (*it)->m_nodeType == NodeBase::Directory ) + { + lock.release(); /// we release the lock because we won't change the children anyways.. + Index::recursiveNodeDealloc( newDir ); + return static_cast< DirectoryNode *>(*it); } + + // replace SubIndex by Directory node. + (*it) = newDir; // and now we are ok to delete the SubIndexNode.. delete subIndex; @@ -717,11 +721,17 @@ DirectoryNode* StreamIndexedIO::Node::directoryChild( const IndexedIO::EntryID & bool StreamIndexedIO::Node::dataChildInfo( const IndexedIO::EntryID &name, size_t &offset, size_t &size ) const { - Index::Mutex::scoped_lock lock( m_idx->m_mutex, false ); // read-only lock - - NodeBase *p = child(name); - if ( p ) + DirectoryNode::ChildMap::const_iterator cit; + if ( m_node->findChild( name, cit ) ) { + Index::Mutex::scoped_lock lock; + if ( m_node->m_subindexChildren ) + { + lock.acquire( m_idx->m_mutex, false ); // read-only lock + } + + NodeBase *p = *cit; + if ( p->m_nodeType == NodeBase::Data ) { DataNode *n = static_cast< DataNode *>( p ); @@ -2211,15 +2221,20 @@ IndexedIO::Entry StreamIndexedIO::entry(const IndexedIO::EntryID &name) const assert( m_node ); readable(name); - Index::Mutex::scoped_lock lock( m_node->m_idx->m_mutex, false ); // read-only lock - - NodeBase* node = m_node->child( name ); - - if (!node) + DirectoryNode::ChildMap::iterator it; + if ( !m_node->m_node->findChild( name, it ) ) { throw IOException( "StreamIndexedIO: Entry not found '" + name.value() + "'" ); } + Index::Mutex::scoped_lock lock; + if ( m_node->m_node->m_subindexChildren ) + { + lock.acquire( m_node->m_idx->m_mutex, false ); // read-only lock + } + + NodeBase *node = *it; + switch( node->m_nodeType ) { case NodeBase::Data: From 96dae627a7cc8f91394823c35fa8665dbc00e1f4 Mon Sep 17 00:00:00 2001 From: Lucio Moser Date: Thu, 27 Feb 2014 16:06:12 -0800 Subject: [PATCH 10/11] Removing conditional on delete. --- src/IECore/StreamIndexedIO.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/IECore/StreamIndexedIO.cpp b/src/IECore/StreamIndexedIO.cpp index f1f001bcad..34193f339a 100644 --- a/src/IECore/StreamIndexedIO.cpp +++ b/src/IECore/StreamIndexedIO.cpp @@ -2021,10 +2021,7 @@ void StreamIndexedIO::open( StreamFilePtr file, const IndexedIO::EntryIDList &ro StreamIndexedIO::~StreamIndexedIO() { - if ( m_node ) - { - delete m_node; - } + delete m_node; // \todo Consider a mechanism for deallocating sub-indexes } From 46a19c67c52ac3f6321b3c0b83323a570f3afa19 Mon Sep 17 00:00:00 2001 From: Lucio Moser Date: Wed, 5 Mar 2014 17:13:25 -0800 Subject: [PATCH 11/11] Protecing member variables from the Node classes. Adding some comments to explain better the rationale behind them. --- src/IECore/StreamIndexedIO.cpp | 577 +++++++++++++++++++-------------- 1 file changed, 341 insertions(+), 236 deletions(-) diff --git a/src/IECore/StreamIndexedIO.cpp b/src/IECore/StreamIndexedIO.cpp index 34193f339a..d09107e93b 100644 --- a/src/IECore/StreamIndexedIO.cpp +++ b/src/IECore/StreamIndexedIO.cpp @@ -119,7 +119,7 @@ IE_CORE_DEFINERUNTIMETYPEDDESCRIPTION( StreamIndexedIO ) //// Templated functions for stream files ////// template -void writeLittleEndian( F &f, T n ) +void writeLittleEndian( F &f, const T &n ) { const T nl = asLittleEndian<>(n); f.write( (const char*) &nl, sizeof(T) ); @@ -291,32 +291,50 @@ class StreamIndexedIO::StringCache mutable unsigned long m_ioBufferLen; }; +/// NodeBase is a base class for nodes representing the index +// It's designed to keep the size of nodes to a minimum for dealing with large indexes +// As a result we deliberately not deriving from RefCounted to save the size of the refcount and the vptr (due to the virtual methods) +// Because it has no virtual destructor, we provide the destroy() function that carefully casts the Node pointer to the appropriate derived type before deleting it. +// NodeType enumerates the derived types, and m_nodeType is defined at construction by the derived classes class NodeBase { public : typedef enum { + Base, SmallData, Data, Directory, SubIndex } NodeType; - NodeBase( IndexedIO::EntryID name ) : m_name(name) {} + NodeBase( NodeType type, IndexedIO::EntryID name ) : m_name(name), m_nodeType(type) {} - NodeBase( NodeType type ) : m_nodeType(type) {} - - // name of the node in the current directory - IndexedIO::EntryID m_name; + inline const IndexedIO::EntryID &name() + { + return m_name; + } - // using char instead of enum to compact members in one word - char m_nodeType; + inline NodeType nodeType() + { + return static_cast(m_nodeType); + } static bool compareNames(const NodeBase* a, const NodeBase* b) { return a->m_name < b->m_name; } + static void destroy( NodeBase *n ); + +protected : + + // name of the node in the current directory + const IndexedIO::EntryID m_name; + + // using char instead of enum to compact members in one word + const char m_nodeType; + }; /// Class that represents small data nodes @@ -329,20 +347,43 @@ class SmallDataNode : public NodeBase static const size_t maxArrayLength = UINT16_MAX; static const size_t maxSize = UINT32_MAX; - SmallDataNode() : NodeBase(NodeBase::SmallData) {} + SmallDataNode( IndexedIO::EntryID name, IndexedIO::DataType dataType, Imf::Int64 arrayLength, Imf::Int64 size, Imf::Int64 offset ) : + NodeBase(NodeBase::SmallData, name), m_dataType(dataType), m_arrayLength((Length)arrayLength), m_size((Size)size), m_offset(offset) {} + + inline IndexedIO::DataType dataType() + { + return static_cast(m_dataType); + } + + inline Imf::Int64 arrayLength() + { + return m_arrayLength; + } + + inline Imf::Int64 size() + { + return m_size; + } + + inline Imf::Int64 offset() + { + return m_offset; + } + + protected : /// data fields from IndexedIO::Entry // using char instead of enum to compact members in one word - char m_dataType; + const char m_dataType; /// data fields from IndexedIO::Entry - Length m_arrayLength; + const Length m_arrayLength; /// The size of this node's data chunk within the file - Size m_size; + const Size m_size; /// The offset in the file to this node's data - Imf::Int64 m_offset; + const Imf::Int64 m_offset; }; /// Class that represents Data nodes @@ -352,7 +393,38 @@ class DataNode : public NodeBase static const size_t maxArrayLength = UINT64_MAX; static const size_t maxSize = UINT64_MAX; - DataNode() : NodeBase(NodeBase::Data) {} + DataNode( IndexedIO::EntryID name, IndexedIO::DataType dataType, Imf::Int64 arrayLength, Imf::Int64 size, Imf::Int64 offset ) : + NodeBase(NodeBase::Data, name), m_dataType(dataType), m_arrayLength(arrayLength), m_size(size), m_offset(offset) {} + + inline IndexedIO::DataType dataType() + { + return m_dataType; + } + + inline Imf::Int64 arrayLength() + { + return m_arrayLength; + } + + inline Imf::Int64 size() + { + return m_size; + } + + inline Imf::Int64 offset() + { + return m_offset; + } + + void copyFrom( DataNode *other ) + { + m_dataType = other->m_dataType; + m_arrayLength = other->m_arrayLength; + m_offset = other->m_offset; + m_size = other->m_size; + } + + protected : /// data fields from IndexedIO::Entry IndexedIO::DataType m_dataType; @@ -365,16 +437,23 @@ class DataNode : public NodeBase /// The offset in the file to this node's data Imf::Int64 m_offset; + }; /// A compressed subindex node class SubIndexNode : public NodeBase { public : - SubIndexNode() : NodeBase(NodeBase::SubIndex) {} + SubIndexNode(IndexedIO::EntryID name, Imf::Int64 offset) : NodeBase(NodeBase::SubIndex, name), m_offset(offset) {} + + inline Imf::Int64 offset() + { + return m_offset; + } + protected : /// The offset in the file to this node's subindex block if m_subindex is not NoSubIndex. - Imf::Int64 m_offset; + const Imf::Int64 m_offset; }; @@ -391,23 +470,43 @@ class DirectoryNode : public NodeBase LoadedSubIndex, }; - DirectoryNode() : NodeBase(NodeBase::Directory), m_subindex(NoSubIndex), m_sortedChildren(false), m_subindexChildren(false), m_offset(0), m_parent(0) {} + typedef std::vector< NodeBase* > ChildMap; - char m_subindex; // using char instead of enum to compact members in one word - bool m_sortedChildren; // same as above - bool m_subindexChildren; // true if one or more children are subindex. Helps avoiding the mutex... + // regular constructor + DirectoryNode(IndexedIO::EntryID name) : NodeBase(NodeBase::Directory, name), m_subindex(NoSubIndex), m_sortedChildren(false), m_subindexChildren(false), m_offset(0), m_parent(0) {} - /// The offset in the file to this node's subindex block if m_subindex is not NoSubIndex. - Imf::Int64 m_offset; + // constructor used when building a directory based on an existing SubIndexNode (because we want to load the contents soon). + DirectoryNode( SubIndexNode *subindex, DirectoryNode *parent ) : NodeBase(NodeBase::Directory, subindex->name()), m_subindex(SavedSubIndex), m_sortedChildren(false), m_subindexChildren(false), m_offset(subindex->offset()), m_parent(parent) {} - /// A pointer to the parent node in the tree - will be NULL for the root node - DirectoryNode* m_parent; + // returns what's the state of this directory, whether it's contents are in a subindex and whether they have been loaded or not. + inline SubIndexMode subindex() + { + return static_cast(m_subindex); + } - /// Sorted list of node's children (DirectoryNode or DataNode/SmallDataNode) - typedef std::vector< NodeBase* > ChildMap; - ChildMap m_children; + inline bool subindexChildren() + { + return m_subindexChildren; + } - public: + inline Imf::Int64 offset() + { + return m_offset; + } + + inline DirectoryNode *parent() + { + return m_parent; + } + + /// Returns the current list of child Nodes. + // This function is not thread-safe because SubIndexNode children can be replaced by DirectoryNodes as the function directoryChild() is called. + // So in order to have safe access, the Index::Mutex m_mutex was created. + // \todo we may want to restrict more the access to the internal children and add the manipulation methods in the class instead. + inline ChildMap &children() + { + return m_children; + } inline void sortChildren() { @@ -418,26 +517,50 @@ class DirectoryNode : public NodeBase } } - template< typename T > - bool findChild( IndexedIO::EntryID name, T &it ) + inline ChildMap::iterator findChild( IndexedIO::EntryID name ) { sortChildren(); - NodeBase search(name); - it = std::lower_bound(m_children.begin(), m_children.end(), &search, NodeBase::compareNames ); - if ( it == m_children.end() ) + NodeBase search(NodeBase::Base, name); + ChildMap::iterator it = std::lower_bound(m_children.begin(), m_children.end(), &search, NodeBase::compareNames ); + if ( it != m_children.end() ) { - return false; + if ( (*it)->name() != name ) + { + return m_children.end(); + } } - return ( (*it)->m_name == name ); + return it; } /// registers a child node in this node void registerChild( NodeBase* c ); void path( IndexedIO::EntryIDList &result ) const; + + // Function that changes the SubIndex Mode to SavedSubIndex and saves memory by deallocating it's children. + void setSubIndexOffset( Imf::Int64 offset ); + + // Indicates to this Directory that it's contents have been retrieved from the subindex. + void recoveredSubIndex(); + + protected : + + char m_subindex; // using char instead of enum to compact members in one word + bool m_sortedChildren; // same as above + bool m_subindexChildren; // true if one or more children are subindex. Helps avoiding the mutex... + + /// The offset in the file to this node's subindex block if m_subindex is not NoSubIndex. + Imf::Int64 m_offset; + + /// A pointer to the parent node in the tree - will be NULL for the root node + DirectoryNode* m_parent; + + /// Sorted list of node's children (DirectoryNode or DataNode/SmallDataNode) + ChildMap m_children; + }; -// holds the private member data for StreamIndexedIO instance and provides high level access to the directory nodes +// holds the private member data for StreamIndexedIO instance and provides high level access to the directory nodes, including thread-safety class StreamIndexedIO::Node { public : @@ -591,13 +714,59 @@ class StreamIndexedIO::Index : public RefCounted /// Returns a newly created Node. template < typename F > NodeBase *readNode( F &f ); - - static void recursiveNodeDealloc( NodeBase *n ); }; /////////////////////////////////////////////// // -// DirectoryNode (begin) +// NodeBase +// +/////////////////////////////////////////////// + +void NodeBase::destroy( NodeBase *n ) +{ + if ( !n ) + { + return; + } + switch( n->nodeType() ) + { + case NodeBase::Directory : + { + DirectoryNode *dn = static_cast< DirectoryNode *>(n); + for (DirectoryNode::ChildMap::const_iterator it = dn->children().begin(); it != dn->children().end(); ++it) + { + destroy( *it ); + } + delete dn; + break; + } + case NodeBase::Data : + { + DataNode *dn = static_cast< DataNode *>(n); + delete dn; + break; + } + case NodeBase::SmallData : + { + SmallDataNode *dn = static_cast< SmallDataNode *>(n); + delete dn; + break; + } + case NodeBase::SubIndex : + { + SubIndexNode *dn = static_cast< SubIndexNode *>(n); + delete dn; + break; + } + default: + throw Exception("Unknown node type!"); + } +} + + +/////////////////////////////////////////////// +// +// DirectoryNode // /////////////////////////////////////////////// @@ -614,7 +783,7 @@ void DirectoryNode::registerChild( NodeBase* c ) throw IOException("StreamIndexedIO: Too many children under the same node!"); } - if ( c->m_nodeType == NodeBase::Directory ) + if ( c->nodeType() == NodeBase::Directory ) { DirectoryNode *childNode = static_cast< DirectoryNode *>(c); if (childNode->m_parent) @@ -623,7 +792,7 @@ void DirectoryNode::registerChild( NodeBase* c ) } childNode->m_parent = this; } - else if ( c->m_nodeType == NodeBase::SubIndex ) + else if ( c->nodeType() == NodeBase::SubIndex ) { m_subindexChildren = true; } @@ -640,6 +809,29 @@ void DirectoryNode::path( IndexedIO::EntryIDList &result ) const } } +void DirectoryNode::setSubIndexOffset( Imf::Int64 offset ) +{ + m_offset = offset; + + // mark this node as a saved in a subindex + m_subindex = DirectoryNode::SavedSubIndex; + + // dealloc all the child nodes + for (DirectoryNode::ChildMap::const_iterator it = m_children.begin(); it != m_children.end(); ++it) + { + NodeBase::destroy( *it ); + } + + // remove all children from this node + m_children.clear(); +} + +void DirectoryNode::recoveredSubIndex() +{ + m_subindex = DirectoryNode::LoadedSubIndex; +} + + /////////////////////////////////////////////// // // StreamIndexedIO::Node (begin) @@ -652,28 +844,28 @@ StreamIndexedIO::Node::Node(Index* index, DirectoryNode *dirNode) : m_idx(index) bool StreamIndexedIO::Node::hasChild( const IndexedIO::EntryID &name ) const { - DirectoryNode::ChildMap::const_iterator cit; - return m_node->findChild( name, cit ); + DirectoryNode::ChildMap::const_iterator cit = m_node->findChild( name ); + return cit != m_node->children().end(); } DirectoryNode* StreamIndexedIO::Node::directoryChild( const IndexedIO::EntryID &name ) const { - DirectoryNode::ChildMap::iterator it; - if ( m_node->findChild( name, it ) ) + DirectoryNode::ChildMap::iterator it = m_node->findChild( name ); + if ( it != m_node->children().end() ) { Index::Mutex::scoped_lock lock; - if ( m_node->m_subindexChildren ) + if ( m_node->subindexChildren() ) { lock.acquire( m_idx->m_mutex, false ); // read-only lock } - if ( (*it)->m_nodeType == NodeBase::Directory ) + if ( (*it)->nodeType() == NodeBase::Directory ) { DirectoryNode *dir = static_cast< DirectoryNode *>( (*it) ); - if ( dir->m_subindex == DirectoryNode::SavedSubIndex ) + if ( dir->subindex() == DirectoryNode::SavedSubIndex ) { - if ( m_node->m_subindexChildren ) + if ( m_node->subindexChildren() ) { lock.release(); /// we will not change the children dictionary, so we release the lock! } @@ -683,14 +875,12 @@ DirectoryNode* StreamIndexedIO::Node::directoryChild( const IndexedIO::EntryID & } return dir; } - else if ( (*it)->m_nodeType == NodeBase::SubIndex ) + else if ( (*it)->nodeType() == NodeBase::SubIndex ) { SubIndexNode *subIndex = static_cast< SubIndexNode *>( (*it) ); - DirectoryNode *newDir = new DirectoryNode; - newDir->m_name = subIndex->m_name; - newDir->m_offset = subIndex->m_offset; - newDir->m_parent = m_node; + // build a Directory that knows it's flushed to a subindex. + DirectoryNode *newDir = new DirectoryNode( subIndex, m_node ); lock.release(); /// we release the lock while loading data.. @@ -700,10 +890,10 @@ DirectoryNode* StreamIndexedIO::Node::directoryChild( const IndexedIO::EntryID & lock.acquire( m_idx->m_mutex, true ); // there's a chance that someone else already replaced the pointer... - if ( (*it)->m_nodeType == NodeBase::Directory ) + if ( (*it)->nodeType() == NodeBase::Directory ) { lock.release(); /// we release the lock because we won't change the children anyways.. - Index::recursiveNodeDealloc( newDir ); + NodeBase::destroy( newDir ); return static_cast< DirectoryNode *>(*it); } @@ -721,29 +911,29 @@ DirectoryNode* StreamIndexedIO::Node::directoryChild( const IndexedIO::EntryID & bool StreamIndexedIO::Node::dataChildInfo( const IndexedIO::EntryID &name, size_t &offset, size_t &size ) const { - DirectoryNode::ChildMap::const_iterator cit; - if ( m_node->findChild( name, cit ) ) + DirectoryNode::ChildMap::const_iterator cit = m_node->findChild( name ); + if ( cit != m_node->children().end() ) { Index::Mutex::scoped_lock lock; - if ( m_node->m_subindexChildren ) + if ( m_node->subindexChildren() ) { lock.acquire( m_idx->m_mutex, false ); // read-only lock } NodeBase *p = *cit; - if ( p->m_nodeType == NodeBase::Data ) + if ( p->nodeType() == NodeBase::Data ) { DataNode *n = static_cast< DataNode *>( p ); - offset = n->m_offset; - size = n->m_size; + offset = n->offset(); + size = n->size(); return true; } - else if ( p->m_nodeType == NodeBase::SmallData ) + else if ( p->nodeType() == NodeBase::SmallData ) { SmallDataNode *n = static_cast< SmallDataNode *>( p ); - offset = n->m_offset; - size = n->m_size; + offset = n->offset(); + size = n->size(); return true; } } @@ -752,7 +942,7 @@ bool StreamIndexedIO::Node::dataChildInfo( const IndexedIO::EntryID &name, size_ DirectoryNode* StreamIndexedIO::Node::addChild( const IndexedIO::EntryID &childName ) { - if ( m_node->m_subindex ) + if ( m_node->subindex() ) { throw Exception( "Cannot modify the file at current location! It was already committed to the file." ); } @@ -762,12 +952,11 @@ DirectoryNode* StreamIndexedIO::Node::addChild( const IndexedIO::EntryID &childN return 0; } - DirectoryNode* child = new DirectoryNode(); + DirectoryNode* child = new DirectoryNode(childName); if ( !child ) { throw Exception( "Failed to allocate node!" ); } - child->m_name = childName; m_idx->m_stringCache.add( childName ); m_node->registerChild( child ); @@ -779,7 +968,7 @@ DirectoryNode* StreamIndexedIO::Node::addChild( const IndexedIO::EntryID &childN void StreamIndexedIO::Node::addDataChild( const IndexedIO::EntryID &childName, IndexedIO::DataType dataType, size_t arrayLen, size_t offset, size_t size ) { - if ( m_node->m_subindex ) + if ( m_node->subindex() ) { throw Exception( "Cannot modify the file at current location! It was already committed to the file." ); } @@ -793,30 +982,20 @@ void StreamIndexedIO::Node::addDataChild( const IndexedIO::EntryID &childName, I if ( arrayLen <= SmallDataNode::maxArrayLength && size <= SmallDataNode::maxSize ) { - SmallDataNode* child = new SmallDataNode; + SmallDataNode* child = new SmallDataNode(childName, dataType, arrayLen, size, offset); if ( !child ) { throw Exception( "Failed to allocate node!" ); } - child->m_name = childName; - child->m_dataType = dataType; - child->m_arrayLength = static_cast(arrayLen); - child->m_offset = offset; - child->m_size = static_cast(size); m_node->registerChild( child ); } else { - DataNode* child = new DataNode; + DataNode* child = new DataNode(childName, dataType, arrayLen, size, offset); if ( !child ) { throw Exception( "Failed to allocate node!" ); } - child->m_name = childName; - child->m_dataType = dataType; - child->m_arrayLength = arrayLen; - child->m_offset = offset; - child->m_size = size; m_node->registerChild( child ); } m_idx->m_hasChanged = true; @@ -824,45 +1003,45 @@ void StreamIndexedIO::Node::addDataChild( const IndexedIO::EntryID &childName, I const IndexedIO::EntryID &StreamIndexedIO::Node::name() const { - return m_node->m_name; + return m_node->name(); } void StreamIndexedIO::Node::childNames( IndexedIO::EntryIDList &names ) const { names.clear(); - names.reserve( m_node->m_children.size() ); - for ( DirectoryNode::ChildMap::const_iterator cit = m_node->m_children.begin(); cit != m_node->m_children.end(); cit++ ) + names.reserve( m_node->children().size() ); + for ( DirectoryNode::ChildMap::const_iterator cit = m_node->children().begin(); cit != m_node->children().end(); cit++ ) { - names.push_back( (*cit)->m_name ); + names.push_back( (*cit)->name() ); } } void StreamIndexedIO::Node::childNames( IndexedIO::EntryIDList &names, IndexedIO::EntryType type ) const { names.clear(); - names.reserve( m_node->m_children.size() ); + names.reserve( m_node->children().size() ); bool typeIsDirectory = ( type == IndexedIO::Directory ); - for ( DirectoryNode::ChildMap::const_iterator cit = m_node->m_children.begin(); cit != m_node->m_children.end(); cit++ ) + for ( DirectoryNode::ChildMap::const_iterator cit = m_node->children().begin(); cit != m_node->children().end(); cit++ ) { NodeBase *cc = *cit; - bool childIsDirectory = ( cc->m_nodeType == NodeBase::Directory || cc->m_nodeType == NodeBase::SubIndex ); + bool childIsDirectory = ( cc->nodeType() == NodeBase::Directory || cc->nodeType() == NodeBase::SubIndex ); if ( typeIsDirectory == childIsDirectory ) { - names.push_back( (*cit)->m_name ); + names.push_back( (*cit)->name() ); } } } void StreamIndexedIO::Node::removeChild( const IndexedIO::EntryID &childName, bool throwException ) { - DirectoryNode::ChildMap::iterator it; - if ( !m_node->findChild( childName, it ) ) + DirectoryNode::ChildMap::iterator it = m_node->findChild( childName ); + if ( it == m_node->children().end() ) { if (throwException) { - throw IOException( "StreamIndexedIO:Node:removeChild: Entry not found '" + childName.value() + "'" ); + throw IOException( "StreamIndexedIO::Node::removeChild: Entry not found '" + childName.value() + "'" ); } return; } @@ -871,7 +1050,7 @@ void StreamIndexedIO::Node::removeChild( const IndexedIO::EntryID &childName, bo m_idx->deallocateWalk(child); - m_node->m_children.erase( it ); + m_node->children().erase( it ); } /////////////////////////////////////////////// @@ -904,53 +1083,12 @@ StreamIndexedIO::Index::~Index() } // dealloc all nodes, starting from root - recursiveNodeDealloc( m_root ); + NodeBase::destroy( m_root ); // dealloc removed nodes for ( std::vector< NodeBase * >::const_iterator it = m_removedNodes.begin(); it != m_removedNodes.end(); it++ ) { - recursiveNodeDealloc( *it ); - } -} - -void StreamIndexedIO::Index::recursiveNodeDealloc( NodeBase *n ) -{ - if ( !n ) - { - return; - } - switch( n->m_nodeType ) - { - case NodeBase::Directory : - { - DirectoryNode *dn = static_cast< DirectoryNode *>(n); - for (DirectoryNode::ChildMap::const_iterator it = dn->m_children.begin(); it != dn->m_children.end(); ++it) - { - recursiveNodeDealloc( *it ); - } - delete dn; - break; - } - case NodeBase::Data : - { - DataNode *dn = static_cast< DataNode *>(n); - delete dn; - break; - } - case NodeBase::SmallData : - { - SmallDataNode *dn = static_cast< SmallDataNode *>(n); - delete dn; - break; - } - case NodeBase::SubIndex : - { - SubIndexNode *dn = static_cast< SubIndexNode *>(n); - delete dn; - break; - } - default: - throw Exception("Unknown node type!"); + NodeBase::destroy( *it ); } } @@ -1021,8 +1159,7 @@ void StreamIndexedIO::Index::openStream() else { // creating a new empty Index - m_root = new DirectoryNode(); - m_root->m_name = IndexedIO::rootName; + m_root = new DirectoryNode(IndexedIO::rootName); m_hasChanged = true; } } @@ -1100,26 +1237,22 @@ NodeBase *StreamIndexedIO::Index::readNodeV4( F &f ) if ( entryType == IndexedIO::File ) { - DataNode *n = new DataNode; - - n->m_name = *id; - n->m_dataType = dataType; - n->m_arrayLength = arrayLength; + Imf::Int64 offset, size; if ( isLink ) // Only version 4 { Imf::Int64 targetNodeId; // load Target Node ID in m_offset readLittleEndian( f,targetNodeId ); - n->m_offset = targetNodeId; + offset = targetNodeId; // we cannot assure that the target node is already loaded, /// so we set size to zero for now and after we set it after the whole index is loaded. - n->m_size = 0; + size = 0; } else { - readLittleEndian( f,n->m_offset ); - readLittleEndian( f,n->m_size ); + readLittleEndian( f,offset ); + readLittleEndian( f,size ); if ( m_version == 4 ) { @@ -1129,21 +1262,19 @@ NodeBase *StreamIndexedIO::Index::readNodeV4( F &f ) readLittleEndian(f,linkCount); } } + DataNode *n = new DataNode( *id, dataType, arrayLength, size, offset ); result = n; } else // Directory { - DirectoryNode *n = new DirectoryNode(); - n->m_name = *id; + DirectoryNode *n = new DirectoryNode( *id ); if ( m_version < 2 ) { - Imf::Int64 size; - readLittleEndian( f, n->m_offset ); + Imf::Int64 offset, size; + readLittleEndian( f, offset ); readLittleEndian( f, size ); } - n->m_offset = 0; - result = n; } @@ -1153,7 +1284,7 @@ NodeBase *StreamIndexedIO::Index::readNodeV4( F &f ) if ( parentId < m_indexToNodeMap.size() ) { parent = static_cast< DirectoryNode * >( m_indexToNodeMap[parentId] ); - if ( parent->m_nodeType != NodeBase::Directory ) + if ( parent->nodeType() != NodeBase::Directory ) { throw IOException("StreamIndexedIO: parent is not a directory!"); } @@ -1209,30 +1340,18 @@ NodeBase *StreamIndexedIO::Index::readNode( F &f ) if ( arrayLength <= SmallDataNode::maxArrayLength && size <= SmallDataNode::maxSize ) { - SmallDataNode *n = new SmallDataNode; - n->m_name = m_stringCache.findById( stringId ); - n->m_dataType = dataType; - n->m_offset = offset; - n->m_arrayLength = static_cast( arrayLength ); - n->m_size = static_cast( size ); + SmallDataNode *n = new SmallDataNode( m_stringCache.findById( stringId ), dataType, arrayLength, size, offset ); return n; } else { - DataNode *n = new DataNode; - n->m_name = m_stringCache.findById( stringId ); - n->m_dataType = dataType; - n->m_offset = offset; - n->m_arrayLength = arrayLength; - n->m_size = size; + DataNode *n = new DataNode( m_stringCache.findById( stringId ), dataType, arrayLength, size, offset ); return n; } } else if ( entryType == IndexedIO::Directory ) { - DirectoryNode *n = new DirectoryNode(); - n->m_name = m_stringCache.findById( stringId ); - n->m_subindex = DirectoryNode::NoSubIndex; + DirectoryNode *n = new DirectoryNode( m_stringCache.findById( stringId ) ); uint32_t nodeCount = 0; readLittleEndian( f, nodeCount ); @@ -1248,9 +1367,9 @@ NodeBase *StreamIndexedIO::Index::readNode( F &f ) } else if ( entryType == SUBINDEX_DIR ) { - SubIndexNode *n = new SubIndexNode(); - n->m_name = m_stringCache.findById( stringId ); - readLittleEndian( f,n->m_offset ); + Imf::Int64 offset; + readLittleEndian( f, offset ); + SubIndexNode *n = new SubIndexNode( m_stringCache.findById( stringId ), offset ); return n; } else @@ -1272,7 +1391,7 @@ void StreamIndexedIO::Index::read( F &f ) /// current file format reading m_root = static_cast< DirectoryNode *>( readNode( f ) ); - if ( m_root->m_nodeType != NodeBase::Directory) + if ( m_root->nodeType() != NodeBase::Directory) { throw Exception( "StreamIndexedIO::Index::read - Root node is not a directory!!" ); } @@ -1291,7 +1410,7 @@ void StreamIndexedIO::Index::read( F &f ) } m_root = static_cast< DirectoryNode *>( m_indexToNodeMap[0] ); - if ( m_root->m_nodeType != NodeBase::Directory) + if ( m_root->nodeType() != NodeBase::Directory) { throw Exception( "StreamIndexedIO::Index::read - Root node is not a directory!!" ); } @@ -1304,7 +1423,7 @@ void StreamIndexedIO::Index::read( F &f ) { continue; } - if ( n->m_nodeType == NodeBase::Directory ) + if ( n->nodeType() == NodeBase::Directory ) { static_cast< DirectoryNode * >(n)->sortChildren(); } @@ -1316,27 +1435,24 @@ void StreamIndexedIO::Index::read( F &f ) for (IndexToNodeMap::const_iterator it = m_indexToNodeMap.begin(); it != m_indexToNodeMap.end(); it++ ) { DataNode *n = static_cast< DataNode *>(*it); - if ( !n || n->m_nodeType != NodeBase::Data ) + if ( !n || n->nodeType() != NodeBase::Data ) { continue; } - if ( !n->m_size ) + if ( !n->size() ) { - Imf::Int64 targetNodeId = n->m_offset; + Imf::Int64 targetNodeId = n->offset(); if ( targetNodeId >= m_indexToNodeMap.size() || !m_indexToNodeMap[targetNodeId] ) { throw IOException("StreamIndexedIO: targetNodeId not found"); } DataNode* targetNode = static_cast< DataNode *>(m_indexToNodeMap[targetNodeId]); - if ( targetNode->m_nodeType != NodeBase::Data ) + if ( targetNode->nodeType() != NodeBase::Data ) { throw IOException("StreamIndexedIO: targetNode if not of type File!" ); } - n->m_dataType = targetNode->m_dataType; - n->m_arrayLength = targetNode->m_arrayLength;; - n->m_offset = targetNode->m_offset; - n->m_size = targetNode->m_size; + n->copyFrom( targetNode ); } } } @@ -1368,19 +1484,19 @@ void StreamIndexedIO::Index::writeDataNode( D *node, F &f ) char t = IndexedIO::File; f.write( &t, sizeof(char) ); - Imf::Int64 id = m_stringCache.find( node->m_name ); + Imf::Int64 id = m_stringCache.find( node->name() ); writeLittleEndian( f, id ); - t = node->m_dataType; + t = node->dataType(); f.write( &t, sizeof(char) ); - if ( IndexedIO::Entry::isArray( static_cast(node->m_dataType) ) ) + if ( IndexedIO::Entry::isArray(node->dataType()) ) { - writeLittleEndian( f, node->m_arrayLength ); + writeLittleEndian( f, node->arrayLength() ); } - writeLittleEndian(f, node->m_offset); - writeLittleEndian(f, node->m_size); + writeLittleEndian(f, node->offset()); + writeLittleEndian(f, node->size()); } template < typename F > @@ -1389,22 +1505,22 @@ void StreamIndexedIO::Index::writeNode( SubIndexNode *node, F &f ) char t = SUBINDEX_DIR; f.write( &t, sizeof(char) ); - Imf::Int64 id = m_stringCache.find( node->m_name ); + Imf::Int64 id = m_stringCache.find( node->name() ); writeLittleEndian( f, id ); - writeLittleEndian(f, node->m_offset); + writeLittleEndian(f, node->offset() ); } template < typename F > void StreamIndexedIO::Index::writeNodeChildren( DirectoryNode *n, F &f ) { - uint32_t nodeCount = n->m_children.size(); + uint32_t nodeCount = n->children().size(); writeLittleEndian( f, nodeCount ); - for (DirectoryNode::ChildMap::const_iterator it = n->m_children.begin(); it != n->m_children.end(); ++it) + for (DirectoryNode::ChildMap::const_iterator it = n->children().begin(); it != n->children().end(); ++it) { NodeBase *p = *it; - switch( p->m_nodeType ) + switch( p->nodeType() ) { case NodeBase::Data : { @@ -1439,15 +1555,15 @@ void StreamIndexedIO::Index::writeNodeChildren( DirectoryNode *n, F &f ) template < typename F > void StreamIndexedIO::Index::writeNode( DirectoryNode *node, F &f ) { - char t = ( node->m_subindex ? SUBINDEX_DIR : IndexedIO::Directory ); + char t = ( node->subindex() ? SUBINDEX_DIR : IndexedIO::Directory ); f.write( &t, sizeof(char) ); - Imf::Int64 id = m_stringCache.find( node->m_name ); + Imf::Int64 id = m_stringCache.find( node->name() ); writeLittleEndian( f, id ); - if ( node->m_subindex ) + if ( node->subindex() ) { - writeLittleEndian(f, node->m_offset); + writeLittleEndian(f, node->offset() ); } else { @@ -1579,7 +1695,7 @@ template< typename D > void StreamIndexedIO::Index::deallocate( D* n ) { assert(n); - assert(n->m_nodeType == NodeBase::Data || n->m_nodeType == NodeBase::SmallData ); + assert(n->nodeType() == NodeBase::Data || n->nodeType() == NodeBase::SmallData ); addFreePage( n->m_offset, n->m_size ); } @@ -1760,16 +1876,16 @@ void StreamIndexedIO::Index::deallocateWalk( NodeBase* n ) // store the pointer for future deallocation m_removedNodes.push_back( n ); - if ( n->m_nodeType == NodeBase::Directory ) + if ( n->nodeType() == NodeBase::Directory ) { DirectoryNode *nn = static_cast< DirectoryNode *>(n); - for (DirectoryNode::ChildMap::const_iterator it = nn->m_children.begin(); it != nn->m_children.end(); ++it) + for (DirectoryNode::ChildMap::const_iterator it = nn->children().begin(); it != nn->children().end(); ++it) { deallocateWalk( *it ); } - nn->m_children.clear(); + nn->children().clear(); } else { @@ -1786,7 +1902,7 @@ void StreamIndexedIO::Index::commitNodeToSubIndex( DirectoryNode *n ) return; } - if ( n->m_subindex == DirectoryNode::NoSubIndex ) + if ( n->subindex() == DirectoryNode::NoSubIndex ) { MemoryStreamSink sink; io::filtering_ostream compressingStream; @@ -1804,19 +1920,8 @@ void StreamIndexedIO::Index::commitNodeToSubIndex( DirectoryNode *n ) sink.get( data, sz ); uint32_t subindexSize = sz; - n->m_offset = writeUniqueData( data, subindexSize, true ); - - // mark this node as a saved in a subindex - n->m_subindex = DirectoryNode::SavedSubIndex; - - // dealloc all the child nodes - for (DirectoryNode::ChildMap::const_iterator it = n->m_children.begin(); it != n->m_children.end(); ++it) - { - recursiveNodeDealloc( *it ); - } - - // remove all children from this node - n->m_children.clear(); + // tell the Directory node that it's contents have been written as a subindex + n->setSubIndexOffset( writeUniqueData( data, subindexSize, true ) ); } } @@ -1825,12 +1930,12 @@ void StreamIndexedIO::Index::readNodeFromSubIndex( DirectoryNode *n ) /// guarantees thread safe access to the file and also to the m_subindex variable StreamFile::MutexLock lock( m_stream->mutex() ); - if ( n->m_subindex == DirectoryNode::LoadedSubIndex ) + if ( n->subindex() == DirectoryNode::LoadedSubIndex ) { return; } - m_stream->seekg( n->m_offset, std::ios::beg ); + m_stream->seekg( n->offset(), std::ios::beg ); uint32_t subindexSize = 0; readLittleEndian( *m_stream, subindexSize ); @@ -1855,7 +1960,7 @@ void StreamIndexedIO::Index::readNodeFromSubIndex( DirectoryNode *n ) } /// mark the node as loaded from subindex - n->m_subindex = DirectoryNode::LoadedSubIndex; + n->recoveredSubIndex(); } /////////////////////////////////////////////// @@ -2187,7 +2292,7 @@ void StreamIndexedIO::removeAll( ) { assert( m_node ); - if ( m_node->m_node->m_subindex ) + if ( m_node->m_node->subindex() ) { throw Exception( "Cannot modify the file at current location! It was already committed to the file." ); } @@ -2205,7 +2310,7 @@ void StreamIndexedIO::remove( const IndexedIO::EntryID &name, bool throwIfNonExi assert( m_node ); writable(name); - if ( m_node->m_node->m_subindex ) + if ( m_node->m_node->subindex() ) { throw Exception( "Cannot modify the file at current location! It was already committed to the file." ); } @@ -2218,37 +2323,37 @@ IndexedIO::Entry StreamIndexedIO::entry(const IndexedIO::EntryID &name) const assert( m_node ); readable(name); - DirectoryNode::ChildMap::iterator it; - if ( !m_node->m_node->findChild( name, it ) ) + DirectoryNode::ChildMap::iterator it = m_node->m_node->findChild( name ); + if ( it == m_node->m_node->children().end() ) { - throw IOException( "StreamIndexedIO: Entry not found '" + name.value() + "'" ); + throw IOException( "StreamIndexedIO::entry: Entry not found '" + name.value() + "'" ); } Index::Mutex::scoped_lock lock; - if ( m_node->m_node->m_subindexChildren ) + if ( m_node->m_node->subindexChildren() ) { lock.acquire( m_node->m_idx->m_mutex, false ); // read-only lock } NodeBase *node = *it; - switch( node->m_nodeType ) + switch( node->nodeType() ) { case NodeBase::Data: { DataNode *dn = static_cast< DataNode * >(node); - return IndexedIO::Entry( dn->m_name, IndexedIO::File, static_cast(dn->m_dataType), dn->m_arrayLength ); + return IndexedIO::Entry( dn->name(), IndexedIO::File, dn->dataType(), dn->arrayLength() ); } case NodeBase::SmallData: { SmallDataNode *dn = static_cast< SmallDataNode * >(node); - return IndexedIO::Entry( dn->m_name, IndexedIO::File, static_cast(dn->m_dataType), dn->m_arrayLength ); + return IndexedIO::Entry( dn->name(), IndexedIO::File, dn->dataType(), dn->arrayLength() ); } case NodeBase::Directory: case NodeBase::SubIndex: - return IndexedIO::Entry( node->m_name, IndexedIO::Directory, IndexedIO::Invalid, 0 ); + return IndexedIO::Entry( node->name(), IndexedIO::Directory, IndexedIO::Invalid, 0 ); default: throw Exception("Invalid node type!"); @@ -2258,7 +2363,7 @@ IndexedIO::Entry StreamIndexedIO::entry(const IndexedIO::EntryID &name) const IndexedIOPtr StreamIndexedIO::parentDirectory() { assert( m_node ); - DirectoryNode *parentNode = m_node->m_node->m_parent; + DirectoryNode *parentNode = m_node->m_node->parent(); if ( !parentNode ) { return NULL; @@ -2270,7 +2375,7 @@ IndexedIOPtr StreamIndexedIO::parentDirectory() ConstIndexedIOPtr StreamIndexedIO::parentDirectory() const { assert( m_node ); - DirectoryNode* parentNode = m_node->m_node->m_parent; + DirectoryNode* parentNode = m_node->m_node->parent(); if ( !parentNode ) { return NULL; @@ -2364,7 +2469,7 @@ void StreamIndexedIO::read(const IndexedIO::EntryID &name, InternedString *&x, u if ( !m_node->dataChildInfo( name, dataOffset, dataSize ) ) { - throw IOException( "StreamIndexedIO: Entry not found '" + name.value() + "'" ); + throw IOException( "StreamIndexedIO::read : Data entry not found '" + name.value() + "'" ); } Imf::Int64 *ids = new Imf::Int64[arrayLength]; @@ -2472,7 +2577,7 @@ void StreamIndexedIO::read(const IndexedIO::EntryID &name, T *&x, unsigned long if ( !m_node->dataChildInfo( name, dataOffset, dataSize ) ) { - throw IOException( "StreamIndexedIO: Entry not found '" + name.value() + "'" ); + throw IOException( "StreamIndexedIO::read: Data entry not found '" + name.value() + "'" ); } StreamIndexedIO::StreamFile &f = streamFile(); @@ -2495,7 +2600,7 @@ void StreamIndexedIO::rawRead(const IndexedIO::EntryID &name, T *&x, unsigned lo if ( !m_node->dataChildInfo( name, dataOffset, dataSize ) ) { - throw IOException( "StreamIndexedIO: Data entry not found '" + name.value() + "'" ); + throw IOException( "StreamIndexedIO::rawRead: Data entry not found '" + name.value() + "'" ); } if (!x) @@ -2521,7 +2626,7 @@ void StreamIndexedIO::read(const IndexedIO::EntryID &name, T &x) const if ( !m_node->dataChildInfo( name, dataOffset, dataSize ) ) { - throw IOException( "StreamIndexedIO: Data entry not found '" + name.value() + "'" ); + throw IOException( "StreamIndexedIO::read Data entry not found '" + name.value() + "'" ); } StreamIndexedIO::StreamFile &f = streamFile(); @@ -2544,7 +2649,7 @@ void StreamIndexedIO::rawRead(const IndexedIO::EntryID &name, T &x) const if ( !m_node->dataChildInfo( name, dataOffset, dataSize ) ) { - throw IOException( "StreamIndexedIO: Data entry not found '" + name.value() + "'" ); + throw IOException( "StreamIndexedIO::rawRead: Data entry not found '" + name.value() + "'" ); } StreamIndexedIO::StreamFile &f = streamFile();