Opened 20 years ago

Last modified 18 years ago

#699 new enhancement

maphash enhancement

Reported by: mturk@… Owned by: sdlime
Priority: high Milestone:
Component: MapServer C Library Version: 4.3
Severity: minor Keywords:
Cc:

Description

Hi,

Simple patch that enhances the iterating trough hash table.
It uses the new iterator object for thread saefty.
The current implementation involves hash lookup for each table element.

Here is the sample how it is used

hashTableIteratorObj iter = msHashIteratorNew(hash_table);
while (iter->current) {
  /* iter->current is the pointer to the hashObj
   * so you can use key and data directly
   */
   printf("key = %s data = %s\n", iter->current->key, iter->current->data);

   /* advance to the next element */
   msHashIteratorNext(iter);

}

Attachments (1)

maphash.patch.txt (9.3 KB ) - added by mturk@… 20 years ago.
maphash enhancemt

Download all attachments as: .zip

Change History (6)

by mturk@…, 20 years ago

Attachment: maphash.patch.txt added

maphash enhancemt

comment:1 by sgillies@…, 20 years ago

Cc: sgillies@… added

comment:2 by sdlime, 20 years ago

Cc: morissette@… added

comment:3 by sdlime, 20 years ago

Cc: warmerdam@… added

comment:4 by fwarmerdam, 20 years ago

I can see where this change avoids the hash lookup on each step of the 
iterator, but (in theory at least) the hash lookup should be a fast operation. 
O(log(n)). 

However, I don't see how this implementation is any more threadsafe than the
old one.  In fact, it seems more fragile in that if something has altered the
hash table (removing the current key for instance) while iterating it is easy
to get into an errant pointer situation.  The old implementation will realize
that the old pointer isn't in the hash anymore, it will gracefully continue on
to the next bucket (though potentially missing some items in the old bucket). 

I would add that introducing the whole hash iterator object adds a significant
chunk of code and complexity for no apparent purpose other than performance.  
The use of an iterator adds an easy opportunity for additional memory leaks. 

So, is the justification for the change a speed improvement?  Is hash iteration
really a performance bottleneck?  The only thing I am aware of the iterator
being used for on hashes is when saving map files.  Not generally a very
performance sensitive area. 

My advice (pending additional justification) would be to not apply this patch.

comment:5 by sgillies@…, 18 years ago

Cc: sgillies@… removed
Note: See TracTickets for help on using tickets.