Skip to content

Commit

Permalink
Improved performance of DcmList::seek_to().
Browse files Browse the repository at this point in the history
Significantly improved the performance of the seek_to() method, in
particular when iterating a very long list using the position of the
items, e.g. a DcmPixelSequence with a huge number of instances of
DcmPixelItem such as in Whole Slide Imaging (WSI) objects. Internally,
the seek_to() method is, e.g., used in the insert(), remove() and
getItem() methods of DcmSequenceOfItems and DcmPixelSequence as well
as in the getElement() and remove() method of DcmItem.

Thanks to GitHub user "reunanen" for the report and proposed patch.

This closes GitHub pull request #114.
  • Loading branch information
jriesmeier committed Jan 17, 2025
1 parent b542556 commit f58412b
Show file tree
Hide file tree
Showing 3 changed files with 168 additions and 82 deletions.
53 changes: 29 additions & 24 deletions dcmdata/include/dcmtk/dcmdata/dclist.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/*
*
* Copyright (C) 1994-2021, OFFIS e.V.
* Copyright (C) 1994-2025, OFFIS e.V.
* All rights reserved. See COPYRIGHT file for details.
*
* This software and supporting documentation were developed by
Expand Down Expand Up @@ -34,20 +34,20 @@ const unsigned long DCM_EndOfListIndex = OFstatic_cast(unsigned long, -1L);

/** helper class maintaining an entry in a DcmList double-linked list
*/
class DCMTK_DCMDATA_EXPORT DcmListNode
class DCMTK_DCMDATA_EXPORT DcmListNode
{

public:
/** constructor
* @param obj object to be maintained by this list node
*/
DcmListNode( DcmObject *obj );
DcmListNode(DcmObject *obj);

/// destructor
~DcmListNode();

/// return pointer to object maintained by this list node
inline DcmObject *value() { return objNodeValue; }
inline DcmObject *value() { return objNodeValue; }

private:
friend class DcmList;
Expand All @@ -61,10 +61,10 @@ class DCMTK_DCMDATA_EXPORT DcmListNode
/// pointer to DcmObject instance maintained by this list entry
DcmObject *objNodeValue;

/// private undefined copy constructor
/// private undefined copy constructor
DcmListNode(const DcmListNode &);

/// private undefined copy assignment operator
/// private undefined copy assignment operator
DcmListNode &operator=(const DcmListNode &);

};
Expand All @@ -89,10 +89,10 @@ typedef enum
} E_ListPos;

/** double-linked list class that maintains pointers to DcmObject instances.
* The remove operation does not delete the object pointed to, however,
* the destructor will delete all elements pointed to
* The remove operation does not delete the object pointed to, however, the
* destructor will delete all elements the list points to.
*/
class DCMTK_DCMDATA_EXPORT DcmList
class DCMTK_DCMDATA_EXPORT DcmList
{
public:
/// constructor
Expand All @@ -105,21 +105,21 @@ class DCMTK_DCMDATA_EXPORT DcmList
* @param obj pointer to object
* @return pointer to object
*/
DcmObject *append( DcmObject *obj );
DcmObject *append(DcmObject *obj);

/** insert object at start of list
* @param obj pointer to object
* @return pointer to object
*/
DcmObject *prepend( DcmObject *obj );
DcmObject *prepend(DcmObject *obj);

/** insert object relative to current position and indicator
* @param obj pointer to object
* @param pos position indicator
* @return pointer to object
*/
DcmObject *insert( DcmObject *obj,
E_ListPos pos = ELP_next );
DcmObject *insert(DcmObject *obj,
const E_ListPos pos = ELP_next);

/** remove current entry from list, return element
* @return pointer to removed element, which is not deleted
Expand All @@ -130,36 +130,36 @@ class DCMTK_DCMDATA_EXPORT DcmList
* @param pos position indicator
* @return pointer to object
*/
DcmObject *get( E_ListPos pos = ELP_atpos );
DcmObject *get(const E_ListPos pos = ELP_atpos);

/** seek within element in list to given position
* (i.e. set current element to given position)
* @param pos position indicator
* @return pointer to new current object
*/
DcmObject *seek( E_ListPos pos = ELP_next );
DcmObject *seek(const E_ListPos pos = ELP_next);

/** seek within element in list to given element index
* (i.e. set current element to given index)
* @param absolute_position position index < card()
* @return pointer to new current object
*/
DcmObject *seek_to(unsigned long absolute_position);
DcmObject *seek_to(const unsigned long absolute_position);

/** Remove and delete all elements from list. Thus, the
* elements' memory is also freed by this operation. The list
* is empty after calling this function.
*/
/** remove and delete all elements from list. Thus, the elements' memory
* is also freed by this operation. The list is empty after calling this
* function.
*/
void deleteAllElements();

/// return cardinality of list
inline unsigned long card() const { return cardinality; }

/// return true if list is empty, false otherwise
inline OFBool empty(void) const { return firstNode == NULL; }
inline OFBool empty() const { return firstNode == NULL; }

/// return true if current node exists, false otherwise
inline OFBool valid(void) const { return currentNode != NULL; }
inline OFBool valid() const { return currentNode != NULL; }

private:
/// pointer to first node in list
Expand All @@ -171,10 +171,15 @@ class DCMTK_DCMDATA_EXPORT DcmList
/// pointer to current node in list
DcmListNode *currentNode;

/// current position in list.
/// The position is maintained in order to avoid O(n) lookup
/// when essentially iterating the elements using seek_to().
unsigned long currentPosition;

/// number of elements in list
unsigned long cardinality;
/// private undefined copy constructor

/// private undefined copy constructor
DcmList &operator=(const DcmList &);

/** private undefined copy assignment operator
Expand Down
Loading

0 comments on commit f58412b

Please sign in to comment.