Skip to content

Commit

Permalink
Merge branch 'google:master' into master
Browse files Browse the repository at this point in the history
  • Loading branch information
shitamo authored Apr 4, 2024
2 parents 8460590 + 212b823 commit e1f338e
Show file tree
Hide file tree
Showing 10 changed files with 178 additions and 207 deletions.
2 changes: 2 additions & 0 deletions src/build_mozc.py
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,8 @@ def ExpandMetaTarget(options, meta_target_name):
'out_win/%s_x64:mozc_win32_build64' % config,
'out_win/%s:mozc_installers_win' % config,
]
else:
raise ValueError('unrecognized platform: %s' % target_platform)

return dependencies + targets

Expand Down
2 changes: 1 addition & 1 deletion src/build_tools/binary_size_checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
# in Megabytes.
EXPECTED_MAXIMUM_SIZES = {
# Distribution package files:
'GoogleJapaneseInput.dmg': 75,
'GoogleJapaneseInput.dmg': 85,
'GoogleJapaneseInput64.msi': 65,
}

Expand Down
1 change: 0 additions & 1 deletion src/engine/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,6 @@ mozc_cc_library(
":spellchecker_interface",
":user_data_manager_interface",
"//base:vlog",
"//base/protobuf:message",
"//converter",
"//converter:converter_interface",
"//converter:immutable_converter_interface",
Expand Down
19 changes: 9 additions & 10 deletions src/engine/data_loader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,7 @@ DataLoader::Response BuildResponse(uint64_t id, EngineReloadRequest request) {
}
} // namespace

std::unique_ptr<DataLoader::ResponseFuture> DataLoader::Build(
uint64_t id) const {
DataLoader::ResponseFuture DataLoader::Build(uint64_t id) const {
EngineReloadRequest request;
// Finds the request associated with `id`.
{
Expand All @@ -192,7 +191,7 @@ std::unique_ptr<DataLoader::ResponseFuture> DataLoader::Build(
std::find_if(requests_.begin(), requests_.end(),
[id](const RequestData &v) { return v.id == id; });
if (it == requests_.end()) {
return std::make_unique<DataLoader::ResponseFuture>([id]() {
return DataLoader::ResponseFuture([id]() {
Response response;
response.id = id;
response.response.set_status(EngineReloadResponse::DATA_MISSING);
Expand All @@ -202,16 +201,15 @@ std::unique_ptr<DataLoader::ResponseFuture> DataLoader::Build(
request = it->request;
}

return std::make_unique<DataLoader::ResponseFuture>(
[id, request = std::move(request)]() {
return BuildResponse(id, request);
});
return DataLoader::ResponseFuture([id, request = std::move(request)]() {
return BuildResponse(id, request);
});
}

void DataLoader::MaybeBuildNewData() {
// Checks if an existing build process, or already using the top request.
if (loader_response_future_ || current_request_id_ == top_request_id_ ||
top_request_id_ == 0) {
if (loader_response_future_.has_value() ||
current_request_id_ == top_request_id_ || top_request_id_ == 0) {
return;
}

Expand All @@ -226,7 +224,8 @@ void DataLoader::MaybeBuildNewData() {
}

bool DataLoader::IsBuildResponseReady() const {
return loader_response_future_ && loader_response_future_->Ready();
return loader_response_future_.has_value() &&
loader_response_future_->Ready();
}

std::unique_ptr<DataLoader::Response>
Expand Down
9 changes: 4 additions & 5 deletions src/engine/data_loader.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
#include <atomic>
#include <cstdint>
#include <memory>
#include <optional>
#include <vector>

#include "absl/base/thread_annotations.h"
Expand Down Expand Up @@ -84,10 +85,8 @@ class DataLoader {

// Builds the new engine associated with `id`.
// This method returns the future object immediately.
// Since BackgroundFuture is not movable/copyable, we wrap it with
// std::unique_ptr. This method doesn't return nullptr. All errors
// are stored in EngineReloadResponse::response::status.
virtual std::unique_ptr<ResponseFuture> Build(uint64_t id) const;
// All errors are stored in EngineReloadResponse::response::status.
virtual ResponseFuture Build(uint64_t id) const;

void Clear();

Expand Down Expand Up @@ -130,7 +129,7 @@ class DataLoader {
// 0 means that no data has been updated yet.
std::atomic<uint64_t> current_request_id_ = 0;

std::unique_ptr<DataLoader::ResponseFuture> loader_response_future_;
std::optional<DataLoader::ResponseFuture> loader_response_future_;
// used only in unittest to perform blocking behavior.
bool always_wait_for_loader_response_future_ = false;
};
Expand Down
56 changes: 24 additions & 32 deletions src/engine/data_loader_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -84,11 +84,10 @@ TEST_P(DataLoaderTest, BasicTest) {
request_.set_magic_number(kMockMagicNumber);

const uint64_t id = loader_.RegisterRequest(request_);
std::unique_ptr<DataLoader::ResponseFuture> response_future =
loader_.Build(id);
DataLoader::ResponseFuture response_future = loader_.Build(id);

response_future->Wait();
const DataLoader::Response &response = response_future->Get();
response_future.Wait();
const DataLoader::Response &response = response_future.Get();

DataManager data_manager;
data_manager.InitFromFile(mock_data_path_, kMockMagicNumber);
Expand Down Expand Up @@ -125,10 +124,9 @@ TEST_P(DataLoaderTest, BasicTest) {
request_.set_magic_number(kMockMagicNumber);
const uint64_t id = loader_.RegisterRequest(request_);

std::unique_ptr<DataLoader::ResponseFuture> response_future =
loader_.Build(id);
response_future->Wait();
const DataLoader::Response &response = response_future->Get();
DataLoader::ResponseFuture response_future = loader_.Build(id);
response_future.Wait();
const DataLoader::Response &response = response_future.Get();

DataManager data_manager;
data_manager.InitFromFile(src_path, kMockMagicNumber);
Expand Down Expand Up @@ -173,11 +171,10 @@ TEST_P(DataLoaderTest, AsyncBuildRepeatedly) {
}
}

std::unique_ptr<DataLoader::ResponseFuture> response_future =
loader_.Build(latest_id);
DataLoader::ResponseFuture response_future = loader_.Build(latest_id);

response_future->Wait();
const DataLoader::Response &response = response_future->Get();
response_future.Wait();
const DataLoader::Response &response = response_future.Get();

DataManager data_manager;
data_manager.InitFromFile(last_path, kMockMagicNumber);
Expand All @@ -203,11 +200,10 @@ TEST_P(DataLoaderTest, AsyncBuildWithoutInstall) {
request_.set_magic_number(kMockMagicNumber);
const uint64_t id = loader_.RegisterRequest(request_);

std::unique_ptr<DataLoader::ResponseFuture> response_future =
loader_.Build(id);
DataLoader::ResponseFuture response_future = loader_.Build(id);

response_future->Wait();
const DataLoader::Response &response = response_future->Get();
response_future.Wait();
const DataLoader::Response &response = response_future.Get();

DataManager data_manager;
data_manager.InitFromFile(mock_data_path_, kMockMagicNumber);
Expand Down Expand Up @@ -242,11 +238,10 @@ TEST_P(DataLoaderTest, AsyncBuildWithInstall) {
request_.set_magic_number(kMockMagicNumber);
const uint64_t id = loader_.RegisterRequest(request_);

std::unique_ptr<DataLoader::ResponseFuture> response_future =
loader_.Build(id);
DataLoader::ResponseFuture response_future = loader_.Build(id);

response_future->Wait();
const DataLoader::Response &response = response_future->Get();
response_future.Wait();
const DataLoader::Response &response = response_future.Get();

// Builder should be ready now.
EXPECT_EQ(response.response.status(), EngineReloadResponse::RELOAD_READY);
Expand Down Expand Up @@ -278,11 +273,10 @@ TEST_P(DataLoaderTest, FailureCaseDataBroken) {
request_.set_magic_number(kMockMagicNumber);
const uint64_t id = loader_.RegisterRequest(request_);

std::unique_ptr<DataLoader::ResponseFuture> response_future =
loader_.Build(id);
DataLoader::ResponseFuture response_future = loader_.Build(id);

response_future->Wait();
const DataLoader::Response &response = response_future->Get();
response_future.Wait();
const DataLoader::Response &response = response_future.Get();

EXPECT_EQ(response.response.status(), EngineReloadResponse::DATA_BROKEN);
EXPECT_FALSE(response.modules);
Expand All @@ -297,11 +291,10 @@ TEST_P(DataLoaderTest, InvalidId) {
const uint64_t id =
loader_.RegisterRequest(request_) + 1; // + 1 to make invalid id.

std::unique_ptr<DataLoader::ResponseFuture> response_future =
loader_.Build(id);
DataLoader::ResponseFuture response_future = loader_.Build(id);

response_future->Wait();
const DataLoader::Response &response = response_future->Get();
response_future.Wait();
const DataLoader::Response &response = response_future.Get();

EXPECT_EQ(response.response.status(), EngineReloadResponse::DATA_MISSING);
EXPECT_FALSE(response.modules);
Expand All @@ -315,11 +308,10 @@ TEST_P(DataLoaderTest, FailureCaseFileDoesNotExist) {
request_.set_magic_number(kMockMagicNumber);

const uint64_t id = loader_.RegisterRequest(request_);
std::unique_ptr<DataLoader::ResponseFuture> response_future =
loader_.Build(id);
DataLoader::ResponseFuture response_future = loader_.Build(id);

response_future->Wait();
const DataLoader::Response &response = response_future->Get();
response_future.Wait();
const DataLoader::Response &response = response_future.Get();

EXPECT_EQ(response.response.status(), EngineReloadResponse::MMAP_FAILURE);
EXPECT_FALSE(response.modules);
Expand Down
14 changes: 6 additions & 8 deletions src/engine/engine.cc
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
#include "absl/status/status.h"
#include "absl/status/statusor.h"
#include "absl/strings/string_view.h"
#include "base/protobuf/message.h"
#include "base/vlog.h"
#include "converter/converter.h"
#include "converter/immutable_converter.h"
Expand Down Expand Up @@ -170,8 +169,8 @@ absl::Status Engine::ReloadModules(std::unique_ptr<engine::Modules> modules,
return Init(std::move(modules), is_mobile);
}

absl::Status Engine::Init(
std::unique_ptr<engine::Modules> modules, bool is_mobile) {
absl::Status Engine::Init(std::unique_ptr<engine::Modules> modules,
bool is_mobile) {
#define RETURN_IF_NULL(ptr) \
do { \
if (!(ptr)) \
Expand Down Expand Up @@ -303,12 +302,11 @@ bool Engine::MaybeReloadEngine(EngineReloadResponse *response) {
*response = loader_response->response;

if (!loader_response->modules ||
response->status() != EngineReloadResponse::RELOAD_READY) {
response->status() != EngineReloadResponse::RELOAD_READY) {
// The loader_response does not contain a valid result.

// This request id causes a critical error.
LOG(ERROR) << "Failure in loading response: "
<< protobuf::Utf8Format(*response);
LOG(ERROR) << "Failure in loading response: " << *response;

// Unregisters the invalid ID and continues to rebuild a new data loader.
loader_->ReportLoadFailure(loader_response->id);
Expand All @@ -320,8 +318,8 @@ bool Engine::MaybeReloadEngine(EngineReloadResponse *response) {
}

// Reloads DataManager.
const bool is_mobile = response->request().engine_type() ==
EngineReloadRequest::MOBILE;
const bool is_mobile =
response->request().engine_type() == EngineReloadRequest::MOBILE;
absl::Status reload_status =
ReloadModules(std::move(loader_response->modules), is_mobile);
if (!reload_status.ok()) {
Expand Down
Loading

0 comments on commit e1f338e

Please sign in to comment.