andreacavalli/rocksdb
0
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Deprecate BlockBasedTableOptions.hash_index_allow_collision=false.

Browse Source 
Summary: Deprecate this one option and delete code and tests that are now superfluous.

Test Plan: all tests pass

Reviewers: igor, yhchiang, IslamAbdelRahman

Reviewed By: IslamAbdelRahman

Subscribers: msalib, leveldb, andrewkr, dhruba

Differential Revision: https://reviews.facebook.net/D55317
This commit is contained in:
sdong 2016-05-20 17:14:38 -07:00
parent 0e77246ba9
commit 1d725ca51d
12 changed files with 25 additions and 472 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
CMakeLists.txt
View File

@ -166,7 +166,6 @@ set(SOURCES
table/block_based_table_factory.cc
table/block_based_table_reader.cc
table/block_builder.cc
table/block_hash_index.cc
table/block_prefix_index.cc
table/bloom_block.cc
table/cuckoo_table_builder.cc
@ -383,7 +382,6 @@ set(TESTS
db/write_callback_test.cc
db/write_controller_test.cc
table/block_based_filter_block_test.cc
table/block_hash_index_test.cc
table/block_test.cc
table/cuckoo_table_builder_test.cc
table/cuckoo_table_reader_test.cc

5
HISTORY.md
View File

@ -1,3 +1,8 @@
# Unreleased
### Public API Change
* Deprecate BlockBaseTableOptions.hash_index_allow_collision=false
# Rocksdb Change Log
## Unreleased
### Public API changes

4
Makefile
View File

@ -274,7 +274,6 @@ TESTS = \
db_io_failure_test \
db_properties_test \
db_table_properties_test \
block_hash_index_test \
autovector_test \
column_family_test \
table_properties_collector_test \
@ -807,9 +806,6 @@ cache_bench: util/cache_bench.o $(LIBOBJECTS) $(TESTUTIL)
memtablerep_bench: db/memtablerep_bench.o $(LIBOBJECTS) $(TESTUTIL)
$(AM_LINK)
block_hash_index_test: table/block_hash_index_test.o $(LIBOBJECTS) $(TESTHARNESS)
$(AM_LINK)
db_stress: tools/db_stress.o $(LIBOBJECTS) $(TESTUTIL)
$(AM_LINK)

6
include/rocksdb/table.h
View File

@ -84,10 +84,8 @@ struct BlockBasedTableOptions {
IndexType index_type = kBinarySearch;
// Influence the behavior when kHashSearch is used.
// if false, stores a precise prefix to block range mapping
// if true, does not store prefix and allows prefix hash collision
// (less memory consumption)
// This option is now deprecated. No matter what value it is set to,
// it will behave as if hash_index_allow_collision=true.
bool hash_index_allow_collision = true;
// Use the specified checksum type. Newly created table files will be

2
src.mk
View File

@ -63,7 +63,6 @@ LIB_SOURCES = \
table/block_based_table_reader.cc \
table/block_builder.cc \
table/block.cc \
table/block_hash_index.cc \
table/block_prefix_index.cc \
table/bloom_block.cc \
table/cuckoo_table_builder.cc \
@ -242,7 +241,6 @@ TEST_BENCH_SOURCES = \
db/write_controller_test.cc \
db/write_callback_test.cc \
table/block_based_filter_block_test.cc \
table/block_hash_index_test.cc \
table/block_test.cc \
table/cuckoo_table_builder_test.cc \
table/cuckoo_table_reader_test.cc \

32
table/block.cc
View File

@ -18,7 +18,6 @@
#include "rocksdb/comparator.h"
#include "table/format.h"
#include "table/block_hash_index.h"
#include "table/block_prefix_index.h"
#include "util/coding.h"
#include "util/logging.h"
@ -92,8 +91,7 @@ void BlockIter::Seek(const Slice& target) {
if (prefix_index_) {
ok = PrefixSeek(target, &index);
} else {
ok = hash_index_ ? HashSeek(target, &index)
: BinarySeek(target, 0, num_restarts_ - 1, &index);
ok = BinarySeek(target, 0, num_restarts_ - 1, &index);
}
if (!ok) {
@ -273,21 +271,6 @@ bool BlockIter::BinaryBlockIndexSeek(const Slice& target, uint32_t* block_ids,
}
}
bool BlockIter::HashSeek(const Slice& target, uint32_t* index) {
assert(hash_index_);
auto restart_index = hash_index_->GetRestartIndex(target);
if (restart_index == nullptr) {
current_ = restarts_;
return false;
}
// the elements in restart_array[index : index + num_blocks]
// are all with same prefix. We'll do binary search in that small range.
auto left = restart_index->first_index;
auto right = restart_index->first_index + restart_index->num_blocks - 1;
return BinarySeek(target, left, right, index);
}
bool BlockIter::PrefixSeek(const Slice& target, uint32_t* index) {
assert(prefix_index_);
uint32_t* block_ids = nullptr;
@ -342,36 +325,27 @@ InternalIterator* Block::NewIterator(const Comparator* cmp, BlockIter* iter,
return NewEmptyInternalIterator();
}
} else {
BlockHashIndex* hash_index_ptr =
total_order_seek ? nullptr : hash_index_.get();
BlockPrefixIndex* prefix_index_ptr =
total_order_seek ? nullptr : prefix_index_.get();
if (iter != nullptr) {
iter->Initialize(cmp, data_, restart_offset_, num_restarts,
hash_index_ptr, prefix_index_ptr);
prefix_index_ptr);
} else {
iter = new BlockIter(cmp, data_, restart_offset_, num_restarts,
hash_index_ptr, prefix_index_ptr);
prefix_index_ptr);
}
}
return iter;
}
void Block::SetBlockHashIndex(BlockHashIndex* hash_index) {
hash_index_.reset(hash_index);
}
void Block::SetBlockPrefixIndex(BlockPrefixIndex* prefix_index) {
prefix_index_.reset(prefix_index);
}
size_t Block::ApproximateMemoryUsage() const {
size_t usage = usable_size();
if (hash_index_) {
usage += hash_index_->ApproximateMemoryUsage();
}
if (prefix_index_) {
usage += prefix_index_->ApproximateMemoryUsage();
}

19
table/block.h
View File

@ -18,7 +18,6 @@
#include "db/pinned_iterators_manager.h"
#include "rocksdb/iterator.h"
#include "rocksdb/options.h"
#include "table/block_hash_index.h"
#include "table/block_prefix_index.h"
#include "table/internal_iterator.h"
@ -29,7 +28,6 @@ namespace rocksdb {
struct BlockContents;
class Comparator;
class BlockIter;
class BlockHashIndex;
class BlockPrefixIndex;
class Block {
@ -71,7 +69,6 @@ class Block {
InternalIterator* NewIterator(const Comparator* comparator,
BlockIter* iter = nullptr,
bool total_order_seek = true);
void SetBlockHashIndex(BlockHashIndex* hash_index);
void SetBlockPrefixIndex(BlockPrefixIndex* prefix_index);
// Report an approximation of how much memory has been used.
@ -82,7 +79,6 @@ class Block {
const char* data_; // contents_.data.data()
size_t size_; // contents_.data.size()
uint32_t restart_offset_; // Offset in data_ of restart array
std::unique_ptr<BlockHashIndex> hash_index_;
std::unique_ptr<BlockPrefixIndex> prefix_index_;
// No copying allowed
@ -100,20 +96,17 @@ class BlockIter : public InternalIterator {
current_(0),
restart_index_(0),
status_(Status::OK()),
hash_index_(nullptr),
prefix_index_(nullptr) {}
BlockIter(const Comparator* comparator, const char* data, uint32_t restarts,
uint32_t num_restarts, BlockHashIndex* hash_index,
BlockPrefixIndex* prefix_index)
uint32_t num_restarts, BlockPrefixIndex* prefix_index)
: BlockIter() {
Initialize(comparator, data, restarts, num_restarts,
hash_index, prefix_index);
Initialize(comparator, data, restarts, num_restarts, prefix_index);
}
void Initialize(const Comparator* comparator, const char* data,
uint32_t restarts, uint32_t num_restarts, BlockHashIndex* hash_index,
BlockPrefixIndex* prefix_index) {
uint32_t restarts, uint32_t num_restarts,
BlockPrefixIndex* prefix_index) {
assert(data_ == nullptr); // Ensure it is called only once
assert(num_restarts > 0); // Ensure the param is valid
@ -123,7 +116,6 @@ class BlockIter : public InternalIterator {
num_restarts_ = num_restarts;
current_ = restarts_;
restart_index_ = num_restarts_;
hash_index_ = hash_index;
prefix_index_ = prefix_index;
}
@ -179,7 +171,6 @@ class BlockIter : public InternalIterator {
IterKey key_;
Slice value_;
Status status_;
BlockHashIndex* hash_index_;
BlockPrefixIndex* prefix_index_;
inline int Compare(const Slice& a, const Slice& b) const {
@ -220,8 +211,6 @@ class BlockIter : public InternalIterator {
uint32_t left, uint32_t right,
uint32_t* index);
bool HashSeek(const Slice& target, uint32_t* index);
bool PrefixSeek(const Slice& target, uint32_t* index);
};

33
table/block_based_table_reader.cc
View File

@ -27,7 +27,6 @@
#include "table/block.h"
#include "table/block_based_filter_block.h"
#include "table/block_based_table_factory.h"
#include "table/block_hash_index.h"
#include "table/block_prefix_index.h"
#include "table/filter_block.h"
#include "table/format.h"
@ -278,28 +277,12 @@ class HashIndexReader : public IndexReader {
return Status::OK();
}
if (!hash_index_allow_collision) {
// TODO: deprecate once hash_index_allow_collision proves to be stable.
BlockHashIndex* hash_index = nullptr;
s = CreateBlockHashIndex(hash_key_extractor,
prefixes_contents.data,
prefixes_meta_contents.data,
&hash_index);
// TODO: log error
if (s.ok()) {
new_index_reader->index_block_->SetBlockHashIndex(hash_index);
new_index_reader->OwnPrefixesContents(std::move(prefixes_contents));
}
} else {
BlockPrefixIndex* prefix_index = nullptr;
s = BlockPrefixIndex::Create(hash_key_extractor,
prefixes_contents.data,
prefixes_meta_contents.data,
&prefix_index);
// TODO: log error
if (s.ok()) {
new_index_reader->index_block_->SetBlockPrefixIndex(prefix_index);
}
BlockPrefixIndex* prefix_index = nullptr;
s = BlockPrefixIndex::Create(hash_key_extractor, prefixes_contents.data,
prefixes_meta_contents.data, &prefix_index);
// TODO: log error
if (s.ok()) {
new_index_reader->index_block_->SetBlockPrefixIndex(prefix_index);
}
return Status::OK();
@ -331,10 +314,6 @@ class HashIndexReader : public IndexReader {
~HashIndexReader() {
}
void OwnPrefixesContents(BlockContents&& prefixes_contents) {
prefixes_contents_ = std::move(prefixes_contents);
}
std::unique_ptr<Block> index_block_;
BlockContents prefixes_contents_;
};

159
table/block_hash_index.cc
View File

@ -1,159 +0,0 @@
// Copyright (c) 2011-present, Facebook, Inc. All rights reserved.
// This source code is licensed under the BSD-style license found in the
// LICENSE file in the root directory of this source tree. An additional grant
// of patent rights can be found in the PATENTS file in the same directory.
#include "table/block_hash_index.h"
#include <algorithm>
#include "rocksdb/comparator.h"
#include "rocksdb/iterator.h"
#include "rocksdb/slice_transform.h"
#include "table/internal_iterator.h"
#include "util/coding.h"
namespace rocksdb {
Status CreateBlockHashIndex(const SliceTransform* hash_key_extractor,
const Slice& prefixes, const Slice& prefix_meta,
BlockHashIndex** hash_index) {
uint64_t pos = 0;
auto meta_pos = prefix_meta;
Status s;
*hash_index = new BlockHashIndex(
hash_key_extractor,
false /* external module manages memory space for prefixes */);
while (!meta_pos.empty()) {
uint32_t prefix_size = 0;
uint32_t entry_index = 0;
uint32_t num_blocks = 0;
if (!GetVarint32(&meta_pos, &prefix_size) ||
!GetVarint32(&meta_pos, &entry_index) ||
!GetVarint32(&meta_pos, &num_blocks)) {
s = Status::Corruption(
"Corrupted prefix meta block: unable to read from it.");
break;
}
Slice prefix(prefixes.data() + pos, prefix_size);
(*hash_index)->Add(prefix, entry_index, num_blocks);
pos += prefix_size;
}
if (s.ok() && pos != prefixes.size()) {
s = Status::Corruption("Corrupted prefix meta block");
}
if (!s.ok()) {
delete *hash_index;
}
return s;
}
BlockHashIndex* CreateBlockHashIndexOnTheFly(
InternalIterator* index_iter, InternalIterator* data_iter,
const uint32_t num_restarts, const Comparator* comparator,
const SliceTransform* hash_key_extractor) {
assert(hash_key_extractor);
auto hash_index = new BlockHashIndex(
hash_key_extractor,
true /* hash_index will copy prefix when Add() is called */);
uint32_t current_restart_index = 0;
std::string pending_entry_prefix;
// pending_block_num == 0 also implies there is no entry inserted at all.
uint32_t pending_block_num = 0;
uint32_t pending_entry_index = 0;
// scan all the entries and create a hash index based on their prefixes.
data_iter->SeekToFirst();
for (index_iter->SeekToFirst();
index_iter->Valid() && current_restart_index < num_restarts;
index_iter->Next()) {
Slice last_key_in_block = index_iter->key();
assert(data_iter->Valid() && data_iter->status().ok());
// scan through all entries within a data block.
while (data_iter->Valid() &&
comparator->Compare(data_iter->key(), last_key_in_block) <= 0) {
auto key_prefix = hash_key_extractor->Transform(data_iter->key());
bool is_first_entry = pending_block_num == 0;
// Keys may share the prefix
if (is_first_entry || pending_entry_prefix != key_prefix) {
if (!is_first_entry) {
bool succeeded = hash_index->Add(
pending_entry_prefix, pending_entry_index, pending_block_num);
if (!succeeded) {
delete hash_index;
return nullptr;
}
}
// update the status.
// needs a hard copy otherwise the underlying data changes all the time.
pending_entry_prefix = key_prefix.ToString();
pending_block_num = 1;
pending_entry_index = current_restart_index;
} else {
// entry number increments when keys share the prefix reside in
// different data blocks.
auto last_restart_index = pending_entry_index + pending_block_num - 1;
assert(last_restart_index <= current_restart_index);
if (last_restart_index != current_restart_index) {
++pending_block_num;
}
}
data_iter->Next();
}
++current_restart_index;
}
// make sure all entries has been scaned.
assert(!index_iter->Valid());
assert(!data_iter->Valid());
if (pending_block_num > 0) {
auto succeeded = hash_index->Add(pending_entry_prefix, pending_entry_index,
pending_block_num);
if (!succeeded) {
delete hash_index;
return nullptr;
}
}
return hash_index;
}
bool BlockHashIndex::Add(const Slice& prefix, uint32_t restart_index,
uint32_t num_blocks) {
auto prefix_to_insert = prefix;
if (kOwnPrefixes) {
auto prefix_ptr = arena_.Allocate(prefix.size());
// MSVC reports C4996 Function call with parameters that may be
// unsafe when using std::copy with a output iterator - pointer
memcpy(prefix_ptr, prefix.data(), prefix.size());
prefix_to_insert = Slice(prefix_ptr, prefix.size());
}
auto result = restart_indices_.insert(
{prefix_to_insert, RestartIndex(restart_index, num_blocks)});
return result.second;
}
const BlockHashIndex::RestartIndex* BlockHashIndex::GetRestartIndex(
const Slice& key) {
auto key_prefix = hash_key_extractor_->Transform(key);
auto pos = restart_indices_.find(key_prefix);
if (pos == restart_indices_.end()) {
return nullptr;
}
return &pos->second;
}
} // namespace rocksdb

86
table/block_hash_index.h
View File

@ -1,86 +0,0 @@
// Copyright (c) 2011-present, Facebook, Inc. All rights reserved.
// This source code is licensed under the BSD-style license found in the
// LICENSE file in the root directory of this source tree. An additional grant
// of patent rights can be found in the PATENTS file in the same directory.
#pragma once
#include <string>
#include <unordered_map>
#include "rocksdb/status.h"
#include "util/arena.h"
#include "util/murmurhash.h"
namespace rocksdb {
class Comparator;
class InternalIterator;
class Slice;
class SliceTransform;
// Build a hash-based index to speed up the lookup for "index block".
// BlockHashIndex accepts a key and, if found, returns its restart index within
// that index block.
class BlockHashIndex {
public:
// Represents a restart index in the index block's restart array.
struct RestartIndex {
explicit RestartIndex(uint32_t _first_index, uint32_t _num_blocks = 1)
: first_index(_first_index), num_blocks(_num_blocks) {}
// For a given prefix, what is the restart index for the first data block
// that contains it.
uint32_t first_index = 0;
// How many data blocks contains this prefix?
uint32_t num_blocks = 1;
};
// @params own_prefixes indicate if we should take care the memory space for
// the `key_prefix`
// passed by Add()
explicit BlockHashIndex(const SliceTransform* hash_key_extractor,
bool own_prefixes)
: hash_key_extractor_(hash_key_extractor), kOwnPrefixes(own_prefixes) {}
// Maps a key to its restart first_index.
// Returns nullptr if the restart first_index is found
const RestartIndex* GetRestartIndex(const Slice& key);
bool Add(const Slice& key_prefix, uint32_t restart_index,
uint32_t num_blocks);
size_t ApproximateMemoryUsage() const {
return arena_.ApproximateMemoryUsage();
}
private:
const SliceTransform* hash_key_extractor_;
std::unordered_map<Slice, RestartIndex, murmur_hash> restart_indices_;
Arena arena_;
bool kOwnPrefixes;
};
// Create hash index by reading from the metadata blocks.
// @params prefixes: a sequence of prefixes.
// @params prefix_meta: contains the "metadata" to of the prefixes.
Status CreateBlockHashIndex(const SliceTransform* hash_key_extractor,
const Slice& prefixes, const Slice& prefix_meta,
BlockHashIndex** hash_index);
// Create hash index by scanning the entries in index as well as the whole
// dataset.
// @params index_iter: an iterator with the pointer to the first entry in a
// block.
// @params data_iter: an iterator that can scan all the entries reside in a
// table.
// @params num_restarts: used for correctness verification.
// @params hash_key_extractor: extract the hashable part of a given key.
// On error, nullptr will be returned.
BlockHashIndex* CreateBlockHashIndexOnTheFly(
InternalIterator* index_iter, InternalIterator* data_iter,
const uint32_t num_restarts, const Comparator* comparator,
const SliceTransform* hash_key_extractor);
} // namespace rocksdb

121
table/block_hash_index_test.cc
View File

@ -1,121 +0,0 @@
// Copyright (c) 2011-present, Facebook, Inc. All rights reserved.
// This source code is licensed under the BSD-style license found in the
// LICENSE file in the root directory of this source tree. An additional grant
// of patent rights can be found in the PATENTS file in the same directory.
#include <map>
#include <memory>
#include <vector>
#include "rocksdb/comparator.h"
#include "rocksdb/iterator.h"
#include "rocksdb/slice_transform.h"
#include "table/block_hash_index.h"
#include "table/internal_iterator.h"
#include "util/testharness.h"
#include "util/testutil.h"
namespace rocksdb {
typedef std::map<std::string, std::string> Data;
class MapIterator : public InternalIterator {
public:
explicit MapIterator(const Data& data) : data_(data), pos_(data_.end()) {}
virtual bool Valid() const override { return pos_ != data_.end(); }
virtual void SeekToFirst() override { pos_ = data_.begin(); }
virtual void SeekToLast() override {
pos_ = data_.end();
--pos_;
}
virtual void Seek(const Slice& target) override {
pos_ = data_.find(target.ToString());
}
virtual void Next() override { ++pos_; }
virtual void Prev() override { --pos_; }
virtual Slice key() const override { return pos_->first; }
virtual Slice value() const override { return pos_->second; }
virtual Status status() const override { return Status::OK(); }
private:
const Data& data_;
Data::const_iterator pos_;
};
class BlockTest : public testing::Test {};
TEST_F(BlockTest, BasicTest) {
const size_t keys_per_block = 4;
const size_t prefix_size = 2;
std::vector<std::string> keys = {/* block 1 */
"0101", "0102", "0103", "0201",
/* block 2 */
"0202", "0203", "0301", "0401",
/* block 3 */
"0501", "0601", "0701", "0801",
/* block 4 */
"0802", "0803", "0804", "0805",
/* block 5 */
"0806", "0807", "0808", "0809", };
Data data_entries;
for (const auto key : keys) {
data_entries.insert({key, key});
}
Data index_entries;
for (size_t i = 3; i < keys.size(); i += keys_per_block) {
// simply ignore the value part
index_entries.insert({keys[i], ""});
}
MapIterator data_iter(data_entries);
MapIterator index_iter(index_entries);
auto prefix_extractor = NewFixedPrefixTransform(prefix_size);
std::unique_ptr<BlockHashIndex> block_hash_index(CreateBlockHashIndexOnTheFly(
&index_iter, &data_iter, static_cast<uint32_t>(index_entries.size()),
BytewiseComparator(), prefix_extractor));
std::map<std::string, BlockHashIndex::RestartIndex> expected = {
{"01xx", BlockHashIndex::RestartIndex(0, 1)},
{"02yy", BlockHashIndex::RestartIndex(0, 2)},
{"03zz", BlockHashIndex::RestartIndex(1, 1)},
{"04pp", BlockHashIndex::RestartIndex(1, 1)},
{"05ww", BlockHashIndex::RestartIndex(2, 1)},
{"06xx", BlockHashIndex::RestartIndex(2, 1)},
{"07pp", BlockHashIndex::RestartIndex(2, 1)},
{"08xz", BlockHashIndex::RestartIndex(2, 3)}, };
const BlockHashIndex::RestartIndex* index = nullptr;
// search existed prefixes
for (const auto& item : expected) {
index = block_hash_index->GetRestartIndex(item.first);
ASSERT_TRUE(index != nullptr);
ASSERT_EQ(item.second.first_index, index->first_index);
ASSERT_EQ(item.second.num_blocks, index->num_blocks);
}
// search non exist prefixes
ASSERT_TRUE(!block_hash_index->GetRestartIndex("00xx"));
ASSERT_TRUE(!block_hash_index->GetRestartIndex("10yy"));
ASSERT_TRUE(!block_hash_index->GetRestartIndex("20zz"));
delete prefix_extractor;
}
} // namespace rocksdb
int main(int argc, char** argv) {
::testing::InitGoogleTest(&argc, argv);
return RUN_ALL_TESTS();
}

26
table/block_test.cc
View File

@ -18,7 +18,6 @@
#include "table/block.h"
#include "table/block_builder.h"
#include "table/format.h"
#include "table/block_hash_index.h"
#include "util/random.h"
#include "util/testharness.h"
#include "util/testutil.h"
@ -159,30 +158,16 @@ void CheckBlockContents(BlockContents contents, const int max_key,
std::unique_ptr<const SliceTransform> prefix_extractor(
NewFixedPrefixTransform(prefix_size));
{
auto iter1 = reader1.NewIterator(nullptr);
auto iter2 = reader1.NewIterator(nullptr);
reader1.SetBlockHashIndex(CreateBlockHashIndexOnTheFly(
iter1, iter2, static_cast<uint32_t>(keys.size()), BytewiseComparator(),
prefix_extractor.get()));
delete iter1;
delete iter2;
}
std::unique_ptr<InternalIterator> hash_iter(
reader1.NewIterator(BytewiseComparator(), nullptr, false));
std::unique_ptr<InternalIterator> regular_iter(
reader2.NewIterator(BytewiseComparator()));
// Seek existent keys
for (size_t i = 0; i < keys.size(); i++) {
hash_iter->Seek(keys[i]);
ASSERT_OK(hash_iter->status());
ASSERT_TRUE(hash_iter->Valid());
regular_iter->Seek(keys[i]);
ASSERT_OK(regular_iter->status());
ASSERT_TRUE(regular_iter->Valid());
Slice v = hash_iter->value();
Slice v = regular_iter->value();
ASSERT_EQ(v.ToString().compare(values[i]), 0);
}
@ -192,9 +177,6 @@ void CheckBlockContents(BlockContents contents, const int max_key,
// return the one that is closest.
for (int i = 1; i < max_key - 1; i += 2) {
auto key = GenerateKey(i, 0, 0, nullptr);
hash_iter->Seek(key);
ASSERT_TRUE(!hash_iter->Valid());
regular_iter->Seek(key);
ASSERT_TRUE(regular_iter->Valid());
}