From 1f2f4c745bb691ebaada336a7d18f698e124e6d8 Mon Sep 17 00:00:00 2001 From: sears Date: Tue, 9 Mar 2010 19:02:54 +0000 Subject: [PATCH] more iterator renaming; also, getnext() -> next_callerFrees() git-svn-id: svn+ssh://svn.corp.yahoo.com/yahoo/yrl/labs/pnuts/code/logstore@679 8dad8b1f-cf64-0410-95b6-bcf113ffbcfe --- logiterators.cpp | 2 +- logiterators.h | 2 +- logstore.h | 93 ++++++++++++++++++------------------ memTreeComponent.h | 103 +++++++++++++++++++++------------------- merger.cpp | 17 +++---- test/check_logtable.cpp | 2 +- 6 files changed, 112 insertions(+), 107 deletions(-) diff --git a/logiterators.cpp b/logiterators.cpp index 0d14ed8..2802ab9 100644 --- a/logiterators.cpp +++ b/logiterators.cpp @@ -104,7 +104,7 @@ void diskTreeIterator::init_helper(TUPLE* key1) } template -TUPLE * diskTreeIterator::getnext() +TUPLE * diskTreeIterator::next_callerFrees() { if(!this->lsmIterator_) { return NULL; } diff --git a/logiterators.h b/logiterators.h index d4879cd..3b75b09 100644 --- a/logiterators.h +++ b/logiterators.h @@ -21,7 +21,7 @@ public: ~diskTreeIterator(); - TUPLE * getnext(); + TUPLE * next_callerFrees(); private: void init_iterators(TUPLE * key1, TUPLE * key2); diff --git a/logstore.h b/logstore.h index bb96c5a..69699ce 100644 --- a/logstore.h +++ b/logstore.h @@ -160,7 +160,7 @@ public: current_[0] = first_iter_->getnext(); for(int i = 1; i < num_iters_; i++) { iters_[i-1] = iters[i-1]; - current_[i] = iters_[i-1]->getnext(); + current_[i] = iters_[i-1]->next_callerFrees(); } } ~mergeManyIterator() { @@ -190,7 +190,7 @@ public: if(last_iter_ == 0) { current_[last_iter_] = first_iter_->getnext(); } else if(last_iter_ != -1){ - current_[last_iter_] = iters_[last_iter_-1]->getnext(); + current_[last_iter_] = iters_[last_iter_-1]->next_callerFrees(); } else { // last call was 'peek' } @@ -224,7 +224,7 @@ public: // advance the iterators that match the tuple we're returning. for(int i = 0; i < num_dups; i++) { TUPLE::freetuple(current_[dups[i]]); // should never be null - current_[dups[i]] = iters_[dups[i]-1]->getnext(); + current_[dups[i]] = iters_[dups[i]-1]->next_callerFrees(); } last_iter_ = min; // mark the min iter to be advance at the next invocation of next(). This saves us a copy in the non-merging case. return ret; @@ -330,10 +330,13 @@ private: logtable * ltable; uint64_t epoch; typedef mergeManyIterator< - typename memTreeComponent::changingMemTreeIterator, - typename memTreeComponent::memTreeIterator, TUPLE> inner_merge_it_t; -// typedef mergeManyIterator, diskTreeIterator, TUPLE> merge_it_t; - typedef mergeManyIterator, TUPLE> merge_it_t; + typename memTreeComponent::revalidatingIterator, + typename memTreeComponent::iterator, + TUPLE> inner_merge_it_t; + typedef mergeManyIterator< + inner_merge_it_t, + diskTreeIterator, + TUPLE> merge_it_t; merge_it_t* merge_it_; @@ -342,50 +345,50 @@ private: bool valid; void revalidate() { if(!valid) { - validate(); + validate(); } else { - assert(epoch == ltable->get_epoch()); + assert(epoch == ltable->get_epoch()); } } - void validate() { - typename memTreeComponent::changingMemTreeIterator * c0_it; - typename memTreeComponent::memTreeIterator *c0_mergeable_it[1]; - diskTreeIterator * disk_it[3]; - epoch = ltable->get_epoch(); - if(last_returned) { - c0_it = new typename memTreeComponent::changingMemTreeIterator(ltable->get_tree_c0(), ltable->getMergeData()->rbtree_mut, last_returned); - c0_mergeable_it[0] = new typename memTreeComponent::memTreeIterator (ltable->get_tree_c0_mergeable(), last_returned); - disk_it[0] = new diskTreeIterator (ltable->get_tree_c1(), *last_returned); - disk_it[1] = new diskTreeIterator (ltable->get_tree_c1_mergeable(), *last_returned); - disk_it[2] = new diskTreeIterator (ltable->get_tree_c2(), *last_returned); - } else if(key) { - c0_it = new typename memTreeComponent::changingMemTreeIterator(ltable->get_tree_c0(), ltable->getMergeData()->rbtree_mut, key); - c0_mergeable_it[0] = new typename memTreeComponent::memTreeIterator (ltable->get_tree_c0_mergeable(), key); - disk_it[0] = new diskTreeIterator (ltable->get_tree_c1(), *key); - disk_it[1] = new diskTreeIterator (ltable->get_tree_c1_mergeable(), *key); - disk_it[2] = new diskTreeIterator (ltable->get_tree_c2(), *key); - } else { - c0_it = new typename memTreeComponent::changingMemTreeIterator(ltable->get_tree_c0(), ltable->getMergeData()->rbtree_mut ); - c0_mergeable_it[0] = new typename memTreeComponent::memTreeIterator (ltable->get_tree_c0_mergeable() ); - disk_it[0] = new diskTreeIterator (ltable->get_tree_c1() ); - disk_it[1] = new diskTreeIterator (ltable->get_tree_c1_mergeable() ); - disk_it[2] = new diskTreeIterator (ltable->get_tree_c2() ); - } + void validate() { + typename memTreeComponent::revalidatingIterator * c0_it; + typename memTreeComponent::iterator *c0_mergeable_it[1]; + diskTreeIterator * disk_it[3]; + epoch = ltable->get_epoch(); + if(last_returned) { + c0_it = new typename memTreeComponent::revalidatingIterator(ltable->get_tree_c0(), ltable->getMergeData()->rbtree_mut, last_returned); + c0_mergeable_it[0] = new typename memTreeComponent::iterator (ltable->get_tree_c0_mergeable(), last_returned); + disk_it[0] = new diskTreeIterator (ltable->get_tree_c1(), *last_returned); + disk_it[1] = new diskTreeIterator (ltable->get_tree_c1_mergeable(), *last_returned); + disk_it[2] = new diskTreeIterator (ltable->get_tree_c2(), *last_returned); + } else if(key) { + c0_it = new typename memTreeComponent::revalidatingIterator(ltable->get_tree_c0(), ltable->getMergeData()->rbtree_mut, key); + c0_mergeable_it[0] = new typename memTreeComponent::iterator (ltable->get_tree_c0_mergeable(), key); + disk_it[0] = new diskTreeIterator (ltable->get_tree_c1(), *key); + disk_it[1] = new diskTreeIterator (ltable->get_tree_c1_mergeable(), *key); + disk_it[2] = new diskTreeIterator (ltable->get_tree_c2(), *key); + } else { + c0_it = new typename memTreeComponent::revalidatingIterator(ltable->get_tree_c0(), ltable->getMergeData()->rbtree_mut ); + c0_mergeable_it[0] = new typename memTreeComponent::iterator (ltable->get_tree_c0_mergeable() ); + disk_it[0] = new diskTreeIterator (ltable->get_tree_c1() ); + disk_it[1] = new diskTreeIterator (ltable->get_tree_c1_mergeable() ); + disk_it[2] = new diskTreeIterator (ltable->get_tree_c2() ); + } - inner_merge_it_t * inner_merge_it = - new inner_merge_it_t(c0_it, c0_mergeable_it, 1, NULL, TUPLE::compare_obj); - merge_it_ = new merge_it_t(inner_merge_it, disk_it, 3, NULL, TUPLE::compare_obj); // XXX Hardcodes comparator, and does not handle merges - if(last_returned) { - TUPLE * junk = merge_it_->peek(); - if(junk && !TUPLE::compare(junk->key(), junk->keylen(), last_returned->key(), last_returned->keylen())) { - // we already returned junk - TUPLE::freetuple(merge_it_->getnext()); - } - } - valid = true; - } + inner_merge_it_t * inner_merge_it = + new inner_merge_it_t(c0_it, c0_mergeable_it, 1, NULL, TUPLE::compare_obj); + merge_it_ = new merge_it_t(inner_merge_it, disk_it, 3, NULL, TUPLE::compare_obj); // XXX Hardcodes comparator, and does not handle merges + if(last_returned) { + TUPLE * junk = merge_it_->peek(); + if(junk && !TUPLE::compare(junk->key(), junk->keylen(), last_returned->key(), last_returned->keylen())) { + // we already returned junk + TUPLE::freetuple(merge_it_->getnext()); + } + } + valid = true; + } }; diff --git a/memTreeComponent.h b/memTreeComponent.h index de6371d..584a65b 100644 --- a/memTreeComponent.h +++ b/memTreeComponent.h @@ -10,33 +10,33 @@ public: static void tearDownTree(rbtree_ptr_t t); -////////////////////////////////////////////////////////////// -// memTreeIterator -///////////////////////////////////////////////////////////// +/////////////////////////////////////////////////////////////// +// Plain iterator; cannot cope with changes to underlying tree +/////////////////////////////////////////////////////////////// - class memTreeIterator + class iterator { private: typedef typename rbtree_t::const_iterator MTITER; public: - memTreeIterator( rbtree_t *s ) + iterator( rbtree_t *s ) : first_(true), done_(s == NULL) { init_iterators(s, NULL, NULL); } - memTreeIterator( rbtree_t *s, TUPLE *&key ) + iterator( rbtree_t *s, TUPLE *&key ) : first_(true), done_(s == NULL) { init_iterators(s, key, NULL); } - ~memTreeIterator() { + ~iterator() { delete it_; delete itend_; } - TUPLE* getnext() { + TUPLE* next_callerFrees() { if(done_) { return NULL; } if(first_) { first_ = 0;} else { (*it_)++; } if(*it_==*itend_) { done_= true; return NULL; } @@ -48,24 +48,24 @@ public: private: void init_iterators(rbtree_t * s, TUPLE * key1, TUPLE * key2) { if(s) { - it_ = key1 ? new MTITER(s->find(key1)) : new MTITER(s->begin()); - itend_ = key2 ? new MTITER(s->find(key2)) : new MTITER(s->end()); - if(*it_ == *itend_) { done_ = true; } - if(key1) { - if(done_) { - // DEBUG("memtree opened at eot\n"); - } else { - // DEBUG("memtree opened key = %s\n", (**it_)->key()); - } - } + it_ = key1 ? new MTITER(s->find(key1)) : new MTITER(s->begin()); + itend_ = key2 ? new MTITER(s->find(key2)) : new MTITER(s->end()); + if(*it_ == *itend_) { done_ = true; } + if(key1) { + if(done_) { + // DEBUG("memtree opened at eot\n"); + } else { + // DEBUG("memtree opened key = %s\n", (**it_)->key()); + } + } } else { - it_ = NULL; - itend_ = NULL; + it_ = NULL; + itend_ = NULL; } } - explicit memTreeIterator() { abort(); } - void operator=(memTreeIterator & t) { abort(); } - int operator-(memTreeIterator & t) { abort(); } + explicit iterator() { abort(); } + void operator=(iterator & t) { abort(); } + int operator-(iterator & t) { abort(); } private: bool first_; bool done_; @@ -73,43 +73,48 @@ public: MTITER *itend_; }; - class changingMemTreeIterator + /////////////////////////////////////////////////////////////// + // Revalidating iterator; automatically copes with changes to underlying tree + /////////////////////////////////////////////////////////////// + + + class revalidatingIterator { private: typedef typename rbtree_t::const_iterator MTITER; public: - changingMemTreeIterator( rbtree_t *s, pthread_mutex_t * rb_mut ) : s_(s), mut_(rb_mut) { + revalidatingIterator( rbtree_t *s, pthread_mutex_t * rb_mut ) : s_(s), mut_(rb_mut) { pthread_mutex_lock(mut_); if(s_->begin() == s_->end()) { - next_ret_ = NULL; + next_ret_ = NULL; } else { - next_ret_ = (*s_->begin())->create_copy(); // the create_copy() calls have to happen before we release mut_... + next_ret_ = (*s_->begin())->create_copy(); // the create_copy() calls have to happen before we release mut_... } pthread_mutex_unlock(mut_); } - changingMemTreeIterator( rbtree_t *s, pthread_mutex_t * rb_mut, TUPLE *&key ) : s_(s), mut_(rb_mut) { + revalidatingIterator( rbtree_t *s, pthread_mutex_t * rb_mut, TUPLE *&key ) : s_(s), mut_(rb_mut) { pthread_mutex_lock(mut_); if(key) { - if(s_->find(key) != s_->end()) { - next_ret_ = (*(s_->find(key)))->create_copy(); - } else if(s_->upper_bound(key) != s_->end()) { - next_ret_ = (*(s_->upper_bound(key)))->create_copy(); - } else { - next_ret_ = NULL; - } + if(s_->find(key) != s_->end()) { + next_ret_ = (*(s_->find(key)))->create_copy(); + } else if(s_->upper_bound(key) != s_->end()) { + next_ret_ = (*(s_->upper_bound(key)))->create_copy(); + } else { + next_ret_ = NULL; + } } else { - if(s_->begin() == s_->end()) { - next_ret_ = NULL; - } else { - next_ret_ = (*s_->begin())->create_copy(); // the create_copy() calls have to happen before we release mut_... - } + if(s_->begin() == s_->end()) { + next_ret_ = NULL; + } else { + next_ret_ = (*s_->begin())->create_copy(); // the create_copy() calls have to happen before we release mut_... + } } // DEBUG("changing mem next ret = %s key = %s\n", next_ret_ ? (const char*)next_ret_->key() : "NONE", key ? (const char*)key->key() : "NULL"); pthread_mutex_unlock(mut_); } - ~changingMemTreeIterator() { + ~revalidatingIterator() { if(next_ret_) datatuple::freetuple(next_ret_); } @@ -117,20 +122,20 @@ public: pthread_mutex_lock(mut_); TUPLE * ret = next_ret_; if(next_ret_) { - if(s_->upper_bound(next_ret_) == s_->end()) { - next_ret_ = 0; - } else { - next_ret_ = (*s_->upper_bound(next_ret_))->create_copy(); - } + if(s_->upper_bound(next_ret_) == s_->end()) { + next_ret_ = 0; + } else { + next_ret_ = (*s_->upper_bound(next_ret_))->create_copy(); + } } pthread_mutex_unlock(mut_); return ret; } private: - explicit changingMemTreeIterator() { abort(); } - void operator=(changingMemTreeIterator & t) { abort(); } - int operator-(changingMemTreeIterator & t) { abort(); } + explicit revalidatingIterator() { abort(); } + void operator=(revalidatingIterator & t) { abort(); } + int operator-(revalidatingIterator & t) { abort(); } rbtree_t *s_; TUPLE * next_ret_; diff --git a/merger.cpp b/merger.cpp index 3e1f9db..05bfbb3 100644 --- a/merger.cpp +++ b/merger.cpp @@ -335,8 +335,8 @@ void* memMergeThread(void*arg) //create the iterators diskTreeIterator *itrA = new diskTreeIterator(ltable->get_tree_c1()->get_root_rec()); // XXX don't want get_root_rec() to be here. - memTreeComponent::memTreeIterator *itrB = - new memTreeComponent::memTreeIterator(ltable->get_tree_c0_mergeable()); + memTreeComponent::iterator *itrB = + new memTreeComponent::iterator(ltable->get_tree_c0_mergeable()); //create a new tree @@ -573,14 +573,14 @@ void merge_iterators(int xid, DataPage *dp = 0; - datatuple *t1 = itrA->getnext(); + datatuple *t1 = itrA->next_callerFrees(); if(t1) { stats->num_tuples_in_large++; stats->bytes_in_large += t1->byte_length(); } datatuple *t2 = 0; - while( (t2=itrB->getnext()) != 0) + while( (t2=itrB->next_callerFrees()) != 0) { stats->num_tuples_in_small++; stats->bytes_in_small += t2->byte_length(); @@ -596,7 +596,7 @@ void merge_iterators(int xid, datatuple::freetuple(t1); stats->num_tuples_out++; //advance itrA - t1 = itrA->getnext(); + t1 = itrA->next_callerFrees(); if(t1) { stats->num_tuples_in_large++; stats->bytes_in_large += t1->byte_length(); @@ -612,7 +612,7 @@ void merge_iterators(int xid, dp = insertTuple(xid, dp, mtuple, ltable, scratch_tree, stats); datatuple::freetuple(t1); - t1 = itrA->getnext(); //advance itrA + t1 = itrA->next_callerFrees(); //advance itrA if(t1) { stats->num_tuples_in_large++; stats->bytes_in_large += t1->byte_length(); @@ -637,7 +637,7 @@ void merge_iterators(int xid, datatuple::freetuple(t1); stats->num_tuples_out++; //advance itrA - t1 = itrA->getnext(); + t1 = itrA->next_callerFrees(); if(t1) { stats->num_tuples_in_large++; stats->bytes_in_large += t1->byte_length(); @@ -672,6 +672,3 @@ insertTuple(int xid, DataPage *dp, datatuple *t, return dp; } - - - diff --git a/test/check_logtable.cpp b/test/check_logtable.cpp index 5c096fb..a532d21 100644 --- a/test/check_logtable.cpp +++ b/test/check_logtable.cpp @@ -121,7 +121,7 @@ void insertProbeIter(size_t NUM_ENTRIES) datatuple *dt=0; - while( (dt=tree_itr.getnext()) != NULL) + while( (dt=tree_itr.next_callerFrees()) != NULL) { assert(dt->keylen() == key_arr[tuplenum].length()+1); assert(dt->datalen() == data_arr[tuplenum].length()+1);