-
Notifications
You must be signed in to change notification settings - Fork 38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Api cleanup + SetManifest for Merger #118
Changes from 5 commits
9bd54ba
2cb5d6c
fa942d4
682d35d
c8a6ac6
a644d72
da3377d
bb528fa
8d9fc19
2d36257
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -109,7 +109,9 @@ class DictionaryCompiler | |
if (params_.count(TEMPORARY_PATH_KEY) == 0) { | ||
params_[TEMPORARY_PATH_KEY] = | ||
boost::filesystem::temp_directory_path().string(); | ||
|
||
} else { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. seems there is the same assignment on line: https://github.com/cliqz-oss/keyvi/pull/118/files#diff-a9c1c844ac65d3f1d74ecee3d93fe047L118 |
||
// set temp path for tpie | ||
initializer_.SetTempDirectory(params_[TEMPORARY_PATH_KEY]); | ||
} | ||
|
||
TRACE("tmp path set to %s", params_[TEMPORARY_PATH_KEY].c_str()); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -51,12 +51,11 @@ namespace fsa { | |
* @returns length of the longest common prefix of given strings | ||
*/ | ||
|
||
inline size_t get_common_prefix_length(const char* first, const char* second) { | ||
inline size_t get_common_prefix_length(const std::string& first, const std::string& second) { | ||
|
||
size_t common_prefix_length = 0; | ||
|
||
while (first[common_prefix_length] == second[common_prefix_length] | ||
&& first[common_prefix_length] != 0) { | ||
while (first[common_prefix_length] == second[common_prefix_length] && common_prefix_length < first.size()) { | ||
++common_prefix_length; | ||
} | ||
return common_prefix_length; | ||
|
@@ -190,9 +189,7 @@ final { | |
void Add(const std::string& input_key, typename ValueStoreT::value_t value = | ||
ValueStoreT::no_value) { | ||
|
||
const char* key = input_key.c_str(); | ||
|
||
size_t commonPrefixLength = get_common_prefix_length(last_key_.c_str(), key); | ||
const size_t commonPrefixLength = get_common_prefix_length(last_key_, input_key); | ||
|
||
// keys are equal, just return | ||
if (commonPrefixLength == input_key.size() && last_key_.size() == input_key.size()) { | ||
|
@@ -203,7 +200,7 @@ final { | |
ConsumeStack(commonPrefixLength); | ||
|
||
// put everything that is not common between the two strings (the suffix) into the stack | ||
FeedStack(commonPrefixLength, input_key.size(), key); | ||
FeedStack(commonPrefixLength, input_key.size(), input_key.c_str()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about changing signature of FeedStack to have param There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nice catch! Feedstack already takes a const std::string&, so an implicit conversion happens here. |
||
|
||
// get value and mark final state | ||
bool no_minimization = false; | ||
|
@@ -220,7 +217,7 @@ final { | |
stack_->UpdateWeights(0, input_key.size() + 1, weight); | ||
} | ||
|
||
last_key_ = key; | ||
last_key_ = input_key; | ||
state_ = generator_state::FEEDING; | ||
} | ||
|
||
|
@@ -231,9 +228,7 @@ final { | |
*/ | ||
void Add(const std::string& input_key, const ValueHandle& handle) { | ||
|
||
const char* key = input_key.c_str(); | ||
|
||
size_t commonPrefixLength = get_common_prefix_length(last_key_.c_str(), key); | ||
const size_t commonPrefixLength = get_common_prefix_length(last_key_, input_key); | ||
|
||
// keys are equal, just return | ||
if (commonPrefixLength == input_key.size() && last_key_.size() == input_key.size()) { | ||
|
@@ -244,7 +239,7 @@ final { | |
ConsumeStack(commonPrefixLength); | ||
|
||
// put everything that is not common between the two strings (the suffix) into the stack | ||
FeedStack(commonPrefixLength, input_key.size(), key); | ||
FeedStack(commonPrefixLength, input_key.size(), input_key.c_str()); | ||
|
||
stack_->InsertFinalState(input_key.size(), handle.value_idx, handle.no_minimization); | ||
|
||
|
@@ -256,7 +251,7 @@ final { | |
stack_->UpdateWeights(0, input_key.size() + 1, handle.weight); | ||
} | ||
|
||
last_key_ = std::move(input_key); | ||
last_key_ = input_key; | ||
state_ = generator_state::FEEDING; | ||
} | ||
|
||
|
@@ -358,7 +353,7 @@ final { | |
internal::SerializationUtils::WriteJsonRecord(stream, pt); | ||
} | ||
|
||
inline void FeedStack(const size_t start, const size_t end, const char* key) { | ||
inline void FeedStack(const size_t start, const size_t end, const std::string& key) { | ||
for (size_t i = start; i < end; ++i) { | ||
uint32_t ukey = | ||
static_cast<uint32_t>(static_cast<unsigned char>(key[i])); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest have this getter without
const
qualifier, otherwise its possible to get the pointer from const Dictionary and modify the internal state from outside.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or we can have a
const
qualifier, but with return typestd::shared_ptr<const Automata>
which assures read-only behaviorThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm, there is no internal state in Automata. So the only state would be the shared pointer itself. I will try it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
http://www.cplusplus.com/reference/memory/static_pointer_cast/