Simplify cache config and bind for use in JS (#359)

Deprecates cacheEnabled parameter to be replaced with cacheSize=0.
Python bindings, Documentation in comments and tests updated to reflect
this change.

Exposes the fields corresponding to cache via embind as a value object.
The equivalent object-based syntax in worker.js allows propagation
from JS.

Fixes: #351
See also: mozilla/firefox-translations#96

bergamot-translator-tests

@@ -1 +1 @@
-Subproject commit 3776609ce5f7a238245e303efaa007b2d5078180
+Subproject commit d03a9d316d40ba45c475018287971523666bf51e

bindings/python/bergamot.cpp

@@ -198,18 +198,15 @@ PYBIND11_MODULE(_bergamot, m) {
       .def("pivot", &ServicePyAdapter::pivot);
 
   py::class_<Service::Config>(m, "ServiceConfig")
-      .def(py::init<>([](size_t numWorkers, bool cacheEnabled, size_t cacheSize, std::string logging) {
+      .def(py::init<>([](size_t numWorkers, size_t cacheSize, std::string logging) {
              Service::Config config;
              config.numWorkers = numWorkers;
-             config.cacheEnabled = cacheEnabled;
              config.cacheSize = cacheSize;
              config.logger.level = logging;
              return config;
            }),
-           py::arg("numWorkers") = 1, py::arg("cacheEnabled") = false, py::arg("cacheSize") = 20000,
-           py::arg("logLevel") = "off")
+           py::arg("numWorkers") = 1, py::arg("cacheSize") = 0, py::arg("logLevel") = "off")
       .def_readwrite("numWorkers", &Service::Config::numWorkers)
-      .def_readwrite("cacheEnabled", &Service::Config::cacheEnabled)
       .def_readwrite("cacheSize", &Service::Config::cacheSize);
 
   py::class_<_Model, std::shared_ptr<_Model>>(m, "TranslationModel");

src/translator/service.cpp

@@ -30,8 +30,8 @@ Response combine(Response &&first, Response &&second) {
   return combined;
 }
 
-std::optional<TranslationCache> makeOptionalCache(bool enabled, size_t size, size_t mutexBuckets) {
-  return enabled ? std::make_optional<TranslationCache>(size, mutexBuckets) : std::nullopt;
+std::optional<TranslationCache> makeOptionalCache(size_t size, size_t mutexBuckets) {
+  return size > 0 ? std::make_optional<TranslationCache>(size, mutexBuckets) : std::nullopt;
 }
 
 }  // namespace
@@ -40,7 +40,7 @@ BlockingService::BlockingService(const BlockingService::Config &config)
     : config_(config),
       requestId_(0),
       batchingPool_(),
-      cache_(makeOptionalCache(config.cacheEnabled, config.cacheSize, /*mutexBuckets = */ 1)),
+      cache_(makeOptionalCache(config.cacheSize, /*mutexBuckets = */ 1)),
       logger_(config.logger) {}
 
 std::vector<Response> BlockingService::translateMultiple(std::shared_ptr<TranslationModel> translationModel,
@@ -133,7 +133,7 @@ AsyncService::AsyncService(const AsyncService::Config &config)
     : requestId_(0),
       config_(config),
       safeBatchingPool_(),
-      cache_(makeOptionalCache(config_.cacheEnabled, config_.cacheSize, /*mutexBuckets=*/config_.numWorkers)),
+      cache_(makeOptionalCache(config_.cacheSize, /*mutexBuckets=*/config_.numWorkers)),
       logger_(config.logger) {
   ABORT_IF(config_.numWorkers == 0, "Number of workers should be at least 1 in a threaded workflow");
   workers_.reserve(config_.numWorkers);

src/translator/service.h

@@ -30,21 +30,17 @@ class AsyncService;
 class BlockingService {
  public:
   struct Config {
-    bool cacheEnabled{false};  ///< Whether to enable cache or not.
-
-    /// Size in History items to be stored in the cache. Loosely corresponds to sentences to
-    /// cache in the real world. Note that cache has a random-eviction policy. The peak
-    /// storage at full occupancy is controlled by this parameter. However, whether we attain
-    /// full occupancy or not is controlled by random factors - specifically how uniformly
-    /// the hash distributes.
-    size_t cacheSize{2000};
+    /// Size in History items to be stored in the cache. A value of 0 means no caching. Loosely corresponds to sentences
+    /// to cache in the real world. Note that cache has a random-eviction policy. The peak storage at full occupancy is
+    /// controlled by this parameter. However, whether we attain full occupancy or not is controlled by random factors -
+    /// specifically how uniformly the hash distributes.
+    size_t cacheSize{0};
 
     Logger::Config logger;  ///< Configurations for logging
 
     template <class App>
     static void addOptions(App &app, Config &config) {
       // Options will come here.
-      app.add_option("--cache-translations", config.cacheEnabled, "Whether to cache translations or not.");
       app.add_option("--cache-size", config.cacheSize, "Number of entries to store in cache.");
       Logger::Config::addOptions(app, config.logger);
     }
@@ -112,16 +108,14 @@ class BlockingService {
 class AsyncService {
  public:
   struct Config {
-    size_t numWorkers{1};      ///< How many worker translation threads to spawn.
-    bool cacheEnabled{false};  ///< Whether to enable cache or not.
-    size_t cacheSize{2000};    ///< Size in History items to be stored in the cache. Loosely corresponds to sentences to
-                               /// cache in the real world.
-    Logger::Config logger;     // Configurations for logging
+    size_t numWorkers{1};   ///< How many worker translation threads to spawn.
+    size_t cacheSize{0};    ///< Size in History items to be stored in the cache. Loosely corresponds to sentences to
+                            /// cache in the real world. A value of 0 means no caching.
+    Logger::Config logger;  // Configurations for logging
 
     template <class App>
     static void addOptions(App &app, Config &config) {
       app.add_option("--cpu-threads", config.numWorkers, "Workers to form translation backend");
-      app.add_option("--cache-translations", config.cacheEnabled, "Whether to cache translations or not.");
       app.add_option("--cache-size", config.cacheSize, "Number of entries to store in cache.");
       Logger::Config::addOptions(app, config.logger);
     }

wasm/bindings/service_bindings.cpp

@@ -69,10 +69,8 @@ EMSCRIPTEN_BINDINGS(translation_model) {
 }
 
 EMSCRIPTEN_BINDINGS(blocking_service_config) {
-  value_object<BlockingService::Config>("BlockingServiceConfig");
-  // .field("name", &BlockingService::Config::name")
-  // The above is a future hook. Note that more will come - for cache, for workspace-size or graph details  limits on
-  // aggregate-batching etc.
+  value_object<BlockingService::Config>("BlockingServiceConfig")
+      .field("cacheSize", &BlockingService::Config::cacheSize);
 }
 
 std::shared_ptr<BlockingService> BlockingServiceFactory(const BlockingService::Config& config) {

wasm/test_page/js/worker.js

@@ -78,7 +78,7 @@ onmessage = async function(e) {
 // Instantiates the Translation Service
 const constructTranslationService = async () => {
   if (!translationService) {
-    var translationServiceConfig = {};
+    var translationServiceConfig = {cacheSize: 20000};
     log(`Creating Translation Service with config: ${translationServiceConfig}`);
     translationService = new Module.BlockingService(translationServiceConfig);
     log(`Translation Service created successfully`);