List:Commits« Previous MessageNext Message »
From:Øystein Grøvlen Date:November 25 2010 8:20am
Subject:Re: bzr commit into mysql-trunk branch (tor.didriksen:3247) WL#1393
View as plain text  
On 11/24/10 04:39 PM, Tor Didriksen wrote:
> On 2010-11-24 14:30, Jorgen Loland wrote:
>> > +template<typename Element_type, typename Key_type>
>> > +void Bounded_queue<Element_type, Key_type>::push(Element_type
>> *element)
>> > +{
>> > + DBUG_ASSERT(is_initialized());
>> > + if (queue_is_full((&m_queue)))
>> > + {
>>
>> JL: Comments like these would be nice:
>>
>> // Generate key and put it on top of the heap
>>
>> > + Key_type **pq_top= reinterpret_cast<Key_type
>> **>(queue_top(&m_queue));
>> > + (*m_keymaker)(m_sort_param, *pq_top, element);
>>
>> // Move key to the right location in the heap.
>
> I don't agree, that's alomost like;
> ++it; // Increment iterator.
>
> we are calling queue_replaced() and queue_insert() respectively.

Since the API for the underlying queue is not very well documented, I 
think there is a need for explaining why this work.  It is not evident 
without knowledge about the implementation of PQ algorithms, what 
queue_replaced is doing. Maybe this should be documented in queues.h, 
instead?

I also think this code would be more evident if it was explained in a 
bit more detail how the extra element in bounded_queue is used for 
replacing.  It is not quite clear from comments that at any point in 
time when the underlying queue is full, it will contain the top-N 
elements so far plus the last element that was checked.

--
Øystein
Thread
bzr commit into mysql-trunk branch (tor.didriksen:3247) WL#1393Tor Didriksen22 Nov
  • Re: bzr commit into mysql-trunk branch (tor.didriksen:3247) WL#1393Jorgen Loland24 Nov
    • Re: bzr commit into mysql-trunk branch (tor.didriksen:3247) WL#1393Tor Didriksen24 Nov
      • Re: bzr commit into mysql-trunk branch (tor.didriksen:3247) WL#1393Øystein Grøvlen25 Nov
        • Re: bzr commit into mysql-trunk branch (tor.didriksen:3247) WL#1393Øystein Grøvlen25 Nov
Re: bzr commit into mysql-trunk branch (tor.didriksen:3247) WL#1393Øystein Grøvlen28 Nov