Mercurial > lbo > hg > leveldb-rs
changeset 263:95fd92fcffcb
table_reader: Clean up TableIterator implementation: Store optional current_block.
This should have been done earlier, but seemed to complicated; after some
proper restructuring of the implementation it now makes more sense overall.
author | Lewin Bormann <lbo@spheniscida.de> |
---|---|
date | Tue, 19 Sep 2017 20:09:20 +0200 |
parents | a0738bf82fa3 |
children | 8ccd35514a29 |
files | src/table_reader.rs |
diffstat | 1 files changed, 51 insertions(+), 54 deletions(-) [+] |
line wrap: on
line diff
--- a/src/table_reader.rs Tue Sep 19 20:07:44 2017 +0200 +++ b/src/table_reader.rs Tue Sep 19 20:09:20 2017 +0200 @@ -183,8 +183,7 @@ /// Iterators read from the file; thus only one iterator can be borrowed (mutably) per scope pub fn iter(&self) -> TableIterator { let iter = TableIterator { - current_block: self.indexblock.iter(), - init: false, + current_block: None, current_block_off: 0, index_block: self.indexblock.iter(), table: self.clone(), @@ -256,12 +255,8 @@ // Instead, reference-counted pointers and locks inside the Table ensure that all // TableIterators still share a table. table: Table, - // We're not using Option<BlockIter>, but instead a separate `init` field. That makes it easier - // working with the current block in the iterator methods (no borrowing annoyance as with - // Option<>) - current_block: BlockIter, + current_block: Option<BlockIter>, current_block_off: usize, - init: bool, index_block: BlockIter, } @@ -281,10 +276,9 @@ // Load the block at `handle` into `self.current_block` fn load_block(&mut self, handle: &[u8]) -> Result<()> { let (new_block_handle, _) = BlockHandle::decode(handle); + let block = self.table.read_block(&new_block_handle)?; - let block = try!(self.table.read_block(&new_block_handle)); - - self.current_block = block.block.iter(); + self.current_block = Some(block.block.iter()); self.current_block_off = new_block_handle.offset(); Ok(()) @@ -293,44 +287,44 @@ impl LdbIterator for TableIterator { fn advance(&mut self) -> bool { - // init essentially means that `current_block` is a data block (it's initially filled with - // an index block as filler). - if self.init { - if self.current_block.advance() { - true - } else { - match self.skip_to_next_entry() { - Ok(true) => self.advance(), - Ok(false) => { - self.reset(); - false - } - // try next block, this might be corruption - Err(_) => self.advance(), - } - } - } else { + // Uninitialized case. + if self.current_block.is_none() { match self.skip_to_next_entry() { - Ok(true) => { - // Only initialize if we got an entry - self.init = true; - self.advance() - } + Ok(true) => return self.advance(), Ok(false) => { self.reset(); - false + return false; } // try next block from index, this might be corruption - Err(_) => self.advance(), + Err(_) => return self.advance(), + } + } + + // Initialized case -- does the current block have more entries? + if let Some(ref mut cb) = self.current_block { + if cb.advance() { + return true; } } + + // If the current block is exhausted, try loading the next block. + self.current_block = None; + match self.skip_to_next_entry() { + Ok(true) => self.advance(), + Ok(false) => { + self.reset(); + false + } + // try next block, this might be corruption + Err(_) => self.advance(), + } } + // A call to valid() after seeking is necessary to ensure that the seek worked (e.g., no error // while reading from disk) fn seek(&mut self, to: &[u8]) { // first seek in index block, rewind by one entry (so we get the next smaller index entry), // then set current_block and seek there - self.index_block.seek(to); let (past_block, handle) = current_key_val(&self.index_block) @@ -338,8 +332,8 @@ if self.table.opt.cmp.cmp(to, &past_block) <= Ordering::Equal { // ok, found right block: continue if let Ok(()) = self.load_block(&handle) { - self.current_block.seek(to); - self.init = true; + // current_block is always set if load_block() returned Ok. + self.current_block.as_mut().unwrap().seek(to); } else { self.reset(); return; @@ -352,42 +346,45 @@ fn prev(&mut self) -> bool { // happy path: current block contains previous entry - if self.current_block.prev() { - true - } else { - // Go back one block and look for the last entry in the previous block - if self.index_block.prev() { - if let Some((_, handle)) = current_key_val(&self.index_block) { - if self.load_block(&handle).is_ok() { - self.current_block.seek_to_last(); - self.current_block.valid() - } else { - self.reset(); - false - } + if let Some(ref mut cb) = self.current_block { + if cb.prev() { + return true; + } + } + + // Go back one block and look for the last entry in the previous block + if self.index_block.prev() { + if let Some((_, handle)) = current_key_val(&self.index_block) { + if self.load_block(&handle).is_ok() { + self.current_block.as_mut().unwrap().seek_to_last(); + self.current_block.as_ref().unwrap().valid() } else { + self.reset(); false } } else { false } + } else { + false } } fn reset(&mut self) { self.index_block.reset(); - self.init = false; + self.current_block = None; } // This iterator is special in that it's valid even before the first call to advance(). It // behaves correctly, though. fn valid(&self) -> bool { - self.init && (self.current_block.valid() || self.index_block.valid()) + self.current_block.is_some() && + (self.current_block.as_ref().unwrap().valid() || self.index_block.valid()) } fn current(&self, key: &mut Vec<u8>, val: &mut Vec<u8>) -> bool { - if self.init { - self.current_block.current(key, val) + if let Some(ref cb) = self.current_block { + cb.current(key, val) } else { false }