Skip to content

Commit

Permalink
Improve performance of N-best generation in dictionary predictor.
Browse files Browse the repository at this point in the history 
A heap is constructed on std::vector<Result> but Result is now not so cheap for
swapping. This CL constructs a heap by using std::vector<const Result *>
to improve performance.

name                                          old cpu/op   new cpu/op   delta
BM_MobileOneCharacterPrediction               51.6ms ± 5%  49.2ms ± 4%  -4.61%  (p=0.000 n=48+54)
BM_MobileOneCharacterSuggestion               49.1ms ± 5%  47.1ms ± 5%  -4.10%  (p=0.000 n=49+60)
BM_MobileOneCharacterAmbiguousPrediction      51.8ms ± 5%  49.3ms ± 6%  -4.84%  (p=0.000 n=49+59)
BM_MobileOneCharacterAmbiguousSuggestion      49.4ms ± 7%  47.1ms ± 6%  -4.68%  (p=0.000 n=48+57)
BM_DesktopAnthyCorpusConversion                19.9s ± 1%   20.4s ± 3%  +2.34%    (p=0.016 n=5+4)
BM_MobileAnthyCorpusConversion                 18.1s ± 2%   18.2s ± 1%    ~       (p=0.690 n=5+5)
BM_DesktopStationPredictionCorpusPrediction    5.82s ± 2%   6.04s ± 4%  +3.75%    (p=0.032 n=5+4)
BM_MobileStationPredictionCorpusPrediction     48.2s ± 0%   48.7s ± 0%  +1.11%    (p=0.008 n=5+5)
BM_DesktopStationPredictionCorpusSuggestion    4.08s ± 0%   4.22s ± 3%  +3.22%    (p=0.029 n=4+4)
BM_MobileStationPredictionCorpusSuggestion     42.4s ± 2%   42.8s ± 2%    ~       (p=0.151 n=5+5)

name                                          old time/op             new time/op             delta
BM_MobileOneCharacterPrediction               51.6ms ± 5%             49.2ms ± 4%  -4.60%        (p=0.000 n=48+54)
BM_MobileOneCharacterSuggestion               49.1ms ± 5%             47.1ms ± 5%  -4.13%        (p=0.000 n=49+60)
BM_MobileOneCharacterAmbiguousPrediction      51.9ms ± 5%             49.4ms ± 6%  -4.85%        (p=0.000 n=49+59)
BM_MobileOneCharacterAmbiguousSuggestion      49.4ms ± 7%             47.1ms ± 6%  -4.72%        (p=0.000 n=48+57)
BM_DesktopAnthyCorpusConversion                19.9s ± 1%              20.4s ± 3%  +2.33%          (p=0.032 n=5+4)
BM_MobileAnthyCorpusConversion                 18.1s ± 2%              18.2s ± 1%    ~             (p=0.690 n=5+5)
BM_DesktopStationPredictionCorpusPrediction    5.82s ± 2%              6.04s ± 4%  +3.76%          (p=0.032 n=5+4)
BM_MobileStationPredictionCorpusPrediction     48.2s ± 0%              48.7s ± 0%  +1.14%          (p=0.008 n=5+5)
BM_DesktopStationPredictionCorpusSuggestion    4.14s ± 5%              4.22s ± 3%    ~             (p=0.111 n=5+4)
BM_MobileStationPredictionCorpusSuggestion     42.4s ± 1%              42.8s ± 2%    ~             (p=0.151 n=5+5)

name                                          old allocs/op           new allocs/op           delta
BM_MobileOneCharacterPrediction                60.8k ± 0%              60.8k ± 0%  +0.08%        (p=0.000 n=59+55)
BM_MobileOneCharacterSuggestion                52.6k ± 0%              52.7k ± 0%  +0.09%        (p=0.000 n=59+60)
BM_MobileOneCharacterAmbiguousPrediction       60.8k ± 0%              60.8k ± 0%  +0.08%        (p=0.000 n=60+60)
BM_MobileOneCharacterAmbiguousSuggestion       52.6k ± 0%              52.7k ± 0%  +0.09%        (p=0.000 n=60+57)
BM_DesktopAnthyCorpusConversion                22.0M ± 0%              22.0M ± 0%    ~             (p=0.444 n=5+5)
BM_MobileAnthyCorpusConversion                 22.4M ± 0%              22.4M ± 0%    ~     (all samples are equal)
BM_DesktopStationPredictionCorpusPrediction    5.43M ± 0%              5.45M ± 0%  +0.33%          (p=0.008 n=5+5)
BM_MobileStationPredictionCorpusPrediction     38.8M ± 0%              38.8M ± 0%  +0.05%          (p=0.029 n=4+4)
BM_DesktopStationPredictionCorpusSuggestion    1.87M ± 0%              1.88M ± 0%  +0.97%          (p=0.008 n=5+5)
BM_MobileStationPredictionCorpusSuggestion     27.4M ± 0%              27.4M ± 0%  +0.07%          (p=0.008 n=5+5)

name                                          old peak-mem(Bytes)/op  new peak-mem(Bytes)/op  delta
BM_MobileOneCharacterPrediction                 928k ± 0%               928k ± 0%    ~     (all samples are equal)
BM_MobileOneCharacterSuggestion                 921k ± 0%               921k ± 0%    ~     (all samples are equal)
BM_MobileOneCharacterAmbiguousPrediction        928k ± 0%               928k ± 0%    ~     (all samples are equal)
BM_MobileOneCharacterAmbiguousSuggestion        921k ± 0%               921k ± 0%    ~     (all samples are equal)
BM_DesktopAnthyCorpusConversion                1.64M ± 0%              1.64M ± 0%    ~             (p=0.444 n=5+5)
BM_MobileAnthyCorpusConversion                 1.42M ± 0%              1.42M ± 0%    ~     (all samples are equal)
BM_DesktopStationPredictionCorpusPrediction    1.35M ± 0%              1.35M ± 0%    ~     (all samples are equal)
BM_MobileStationPredictionCorpusPrediction     2.21M ± 0%              2.21M ± 0%    ~     (all samples are equal)
BM_DesktopStationPredictionCorpusSuggestion     648k ± 0%               648k ± 0%    ~     (all samples are equal)
BM_MobileStationPredictionCorpusSuggestion     1.31M ± 0%              1.31M ± 0%    ~             (p=0.444 n=5+5)

PiperOrigin-RevId: 543656867
  • Loading branch information
@hiroyuki-komatsu
Noriyuki Takahashi authored and hiroyuki-komatsu committed Jun 27, 2023
1 parent d55a6a0 commit be09920
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 24 deletions.
1 change: 1 addition & 0 deletions src/prediction/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,7 @@ mozc_cc_test(
"//usage_stats",
"//usage_stats:usage_stats_testing_util",
"@com_google_absl//absl/strings",
"@com_google_absl//absl/types:span",
],
)

Expand Down
32 changes: 18 additions & 14 deletions src/prediction/dictionary_predictor.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,8 @@ bool DictionaryPredictor::PredictForRequest(const ConversionRequest &request,
// Currently, we don't have spelling correction feature when in
// the mixed conversion mode, so RemoveMissSpelledCandidates() is
// not called.
return AddPredictionToCandidates(request, segments, &results);
return AddPredictionToCandidates(request, segments,
absl::MakeSpan(results));
}

// Normal prediction.
Expand All @@ -314,41 +315,44 @@ bool DictionaryPredictor::PredictForRequest(const ConversionRequest &request,
const std::string &input_key = segments->conversion_segment(0).key();
const size_t input_key_len = Util::CharsLen(input_key);
RemoveMissSpelledCandidates(input_key_len, &results);
return AddPredictionToCandidates(request, segments, &results);
return AddPredictionToCandidates(request, segments, absl::MakeSpan(results));
}

bool DictionaryPredictor::AddPredictionToCandidates(
const ConversionRequest &request, Segments *segments,
std::vector<Result> *results) const {
absl::Span<Result> results) const {
DCHECK(segments);
DCHECK(results);

std::string history_key, history_value;
GetHistoryKeyAndValue(*segments, &history_key, &history_value);

Segment *segment = segments->mutable_conversion_segment(0);
DCHECK(segment);

// This pointer array is used to perform heap operations efficiently.
std::vector<const Result *> result_ptrs;
result_ptrs.reserve(results.size());
for (const auto &r : results) result_ptrs.push_back(&r);

// Instead of sorting all the results, we construct a heap.
// This is done in linear time and
// we can pop as many results as we need efficiently.
auto min_heap_cmp = [](const Result &lhs, const Result &rhs) {
auto min_heap_cmp = [](const Result *lhs, const Result *rhs) {
// `rhs < lhs` instead of `lhs < rhs`, since `make_heap()` creates max heap
// by default.
return ResultCostLess()(rhs, lhs);
return ResultCostLess()(*rhs, *lhs);
};
std::make_heap(results->begin(), results->end(), min_heap_cmp);
std::make_heap(result_ptrs.begin(), result_ptrs.end(), min_heap_cmp);

const size_t max_candidates_size = std::min(
request.max_dictionary_prediction_candidates_size(), results->size());
request.max_dictionary_prediction_candidates_size(), results.size());

ResultFilter filter(request, *segments, suggestion_filter_);
int added = 0;

// TODO(taku): Sets more advanced debug info depending on the verbose_level.
absl::flat_hash_map<std::string, int32_t> merged_types;
if (IsDebug(request)) {
for (const auto &result : *results) {
for (const auto &result : results) {
if (!result.removed) {
merged_types[result.value] |= result.types;
}
Expand All @@ -373,10 +377,10 @@ bool DictionaryPredictor::AddPredictionToCandidates(

#endif // MOZC_DEBUG

for (size_t i = 0; i < results->size(); ++i) {
// Pop a result from a heap. Please pay attention not to use results->at(i).
std::pop_heap(results->begin(), results->end() - i, min_heap_cmp);
const Result &result = results->at(results->size() - i - 1);
int added = 0;
for (size_t i = 0; i < result_ptrs.size(); ++i) {
std::pop_heap(result_ptrs.begin(), result_ptrs.end() - i, min_heap_cmp);
const Result &result = *result_ptrs[result_ptrs.size() - i - 1];

if (added >= max_candidates_size || result.cost >= kInfinity) {
break;
Expand Down
2 changes: 1 addition & 1 deletion src/prediction/dictionary_predictor.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ class DictionaryPredictor : public PredictorInterface {

bool AddPredictionToCandidates(const ConversionRequest &request,
Segments *segments,
std::vector<Result> *results) const;
absl::Span<Result> results) const;

void FillCandidate(
const ConversionRequest &request, const Result &result,
Expand Down
19 changes: 10 additions & 9 deletions src/prediction/dictionary_predictor_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
#include "absl/strings/match.h"
#include "absl/strings/str_cat.h"
#include "absl/strings/string_view.h"
#include "absl/types/span.h"

namespace mozc::prediction {

Expand Down Expand Up @@ -136,7 +137,7 @@ class DictionaryPredictorTestPeer {

bool AddPredictionToCandidates(const ConversionRequest &request,
Segments *segments,
std::vector<Result> *results) const {
absl::Span<Result> results) const {
return predictor_.AddPredictionToCandidates(request, segments, results);
}

Expand Down Expand Up @@ -1007,7 +1008,7 @@ TEST_F(DictionaryPredictorTest, MergeAttributesForDebug) {
// Enables debug mode.
config_->set_verbose_level(1);
predictor.AddPredictionToCandidates(*convreq_for_suggestion_, &segments,
&results);
absl::MakeSpan(results));

EXPECT_EQ(segments.conversion_segments_size(), 1);
const Segment &segment = segments.conversion_segment(0);
Expand All @@ -1033,7 +1034,7 @@ TEST_F(DictionaryPredictorTest, SetDescription) {
InitSegmentsWithKey("test", &segments);

predictor.AddPredictionToCandidates(*convreq_for_prediction_, &segments,
&results);
absl::MakeSpan(results));

EXPECT_EQ(segments.conversion_segments_size(), 1);
const Segment &segment = segments.conversion_segment(0);
Expand Down Expand Up @@ -1075,7 +1076,7 @@ TEST_F(DictionaryPredictorTest, PropagateResultCosts) {
kTestSize);

predictor.AddPredictionToCandidates(*convreq_for_suggestion_, &segments,
&results);
absl::MakeSpan(results));

EXPECT_EQ(segments.conversion_segments_size(), 1);
ASSERT_EQ(kTestSize, segments.conversion_segment(0).candidates_size());
Expand Down Expand Up @@ -1115,7 +1116,7 @@ TEST_F(DictionaryPredictorTest, PredictNCandidates) {
kLowCostCandidateSize + 1);

predictor.AddPredictionToCandidates(*convreq_for_suggestion_, &segments,
&results);
absl::MakeSpan(results));

ASSERT_EQ(1, segments.conversion_segments_size());
ASSERT_EQ(kLowCostCandidateSize,
Expand Down Expand Up @@ -1500,7 +1501,7 @@ TEST_F(DictionaryPredictorTest, Dedup) {
Segments segments;
InitSegmentsWithKey("test", &segments);
predictor.AddPredictionToCandidates(*convreq_for_prediction_, &segments,
&results);
absl::MakeSpan(results));

ASSERT_EQ(segments.conversion_segments_size(), 1);
EXPECT_EQ(segments.conversion_segment(0).candidates_size(), kSize);
Expand All @@ -1527,7 +1528,7 @@ TEST_F(DictionaryPredictorTest, Dedup) {
Segments segments;
InitSegmentsWithKey("test", &segments);
predictor.AddPredictionToCandidates(*convreq_for_prediction_, &segments,
&results);
absl::MakeSpan(results));

ASSERT_EQ(segments.conversion_segments_size(), 1);
EXPECT_EQ(segments.conversion_segment(0).candidates_size(), 3);
Expand Down Expand Up @@ -1569,7 +1570,7 @@ TEST_F(DictionaryPredictorTest, TypingCorrectionResultsLimit) {
Segments segments;
InitSegmentsWithKey("original_key", &segments);
predictor.AddPredictionToCandidates(*convreq_for_prediction_, &segments,
&results);
absl::MakeSpan(results));
ASSERT_EQ(segments.conversion_segments_size(), 1);
const Segment segment = segments.conversion_segment(0);
EXPECT_EQ(segment.candidates_size(), 4);
Expand Down Expand Up @@ -1604,7 +1605,7 @@ TEST_F(DictionaryPredictorTest, SortResult) {
Segments segments;
InitSegmentsWithKey("test", &segments);
predictor.AddPredictionToCandidates(*convreq_for_prediction_, &segments,
&results);
absl::MakeSpan(results));

ASSERT_EQ(segments.conversion_segments_size(), 1);
const Segment &segment = segments.conversion_segment(0);
Expand Down

0 comments on commit be09920

Please sign in to comment.