diff --git a/src/user/user_cache.cc b/src/user/user_cache.cc index 6c97060f..4711a55c 100644 --- a/src/user/user_cache.cc +++ b/src/user/user_cache.cc @@ -187,16 +187,23 @@ const std::string* mjCCache::HasAsset(const std::string& id) { // is updated only if the timestamps disagree bool mjCCache::Insert(const mjCAsset& asset) { std::lock_guard lock(mutex_); + + // check if asset is too large to fit in the cache + std::size_t nbytes = asset.BytesCount(); const std::string& id = asset.Id(); + if ((size_ + nbytes > max_size_) && lookup_.find(id) == lookup_.end()) { + return false; + } + if (asset.References().size() != 1) { return false; } const std::string& filename = *(asset.References().begin()); auto [it, inserted] = lookup_.insert({id, asset}); + mjCAsset* asset_ptr = &(it->second); if (!inserted) { - mjCAsset* asset_ptr = &(it->second); - if (size_ - asset_ptr->BytesCount() + asset.BytesCount() > max_size_) { + if (size_ - asset_ptr->BytesCount() + nbytes > max_size_) { return false; } models_[filename].insert(asset_ptr); // add it for the model @@ -205,19 +212,16 @@ bool mjCCache::Insert(const mjCAsset& asset) { return true; } asset_ptr->SetTimestamp(asset.Timestamp()); - size_ = size_ - asset_ptr->BytesCount() + asset.BytesCount(); - asset_ptr->ReplaceBlocks(asset.Blocks(), asset.BytesCount()); + size_ = size_ - asset_ptr->BytesCount() + nbytes; + asset_ptr->ReplaceBlocks(asset.Blocks(), nbytes); return true; - } else if (size_ + asset.BytesCount() > max_size_) { - return false; } // new asset - mjCAsset* asset_ptr = &(it->second); asset_ptr->SetInsertNum(insert_num_++); entries_.insert(asset_ptr); models_[filename].insert(asset_ptr); - size_ += asset.BytesCount(); + size_ += nbytes; return true; } @@ -225,16 +229,22 @@ bool mjCCache::Insert(const mjCAsset& asset) { bool mjCCache::Insert(mjCAsset&& asset) { std::lock_guard lock(mutex_); + + // check if asset is too large to fit in the cache + std::size_t nbytes = asset.BytesCount(); const std::string& id = asset.Id(); + if ((size_ + nbytes > max_size_) && lookup_.find(id) == lookup_.end()) { + return false; + } + if (asset.References().size() != 1) { return false; } const std::string& filename = *(asset.References().begin()); - std::size_t nbytes = asset.BytesCount(); auto [it, inserted] = lookup_.try_emplace(id, std::move(asset)); + mjCAsset* asset_ptr = &(it->second); if (!inserted) { - mjCAsset* asset_ptr = &(it->second); if (size_ - asset_ptr->BytesCount() + nbytes > max_size_) { return false; } @@ -248,12 +258,9 @@ bool mjCCache::Insert(mjCAsset&& asset) { size_ = size_ - asset_ptr->BytesCount() + nbytes; asset_ptr->ReplaceBlocks(std::move(asset.blocks_), asset.nbytes_); return true; - } else if (size_ + nbytes > max_size_) { - return false; } // new asset - mjCAsset* asset_ptr = &(it->second); asset_ptr->SetInsertNum(insert_num_++); entries_.insert(asset_ptr); models_[filename].insert(asset_ptr); diff --git a/test/user/user_cache_test.cc b/test/user/user_cache_test.cc index ed8b3ff4..99c513ae 100644 --- a/test/user/user_cache_test.cc +++ b/test/user/user_cache_test.cc @@ -232,6 +232,43 @@ TEST(CacheTest, LimitTest2) { EXPECT_THAT(cache.HasAsset("bar.obj"), NotNull()); } +// stress test with large asset +TEST(CacheTest, LimitTest3) { + mjCCache cache(12); + std::vector v1 = {1, 2, 3}; + std::vector v2 = {1, 2, 3, 4, 5}; + mjCAsset asset1("file.xml", "foo.obj", "now"); + mjCAsset asset2("file.xml", "bar.obj", "now"); + asset1.AddVector("v", v1); + asset2.AddVector("v", v2); + + cache.Insert(std::move(asset1)); + cache.Insert(std::move(asset2)); + + // foo should still be in cache + EXPECT_THAT(cache.HasAsset("foo.obj"), NotNull()); + + // bar could not be inserted because it's too large + EXPECT_THAT(cache.HasAsset("bar.obj"), IsNull()); +} + +TEST(CacheTest, LimitTest4) { + mjCCache cache(12); + std::vector v = {1, 2, 3}; + mjCAsset asset1("file.xml", "foo.obj", "now"); + mjCAsset asset2("file.xml", "bar.obj", "now"); + asset1.AddVector("v", v); + asset2.AddVector("v", v); + + cache.Insert(std::move(asset1)); + cache.Insert(std::move(asset2)); + + EXPECT_THAT(cache.HasAsset("foo.obj"), NotNull()); + + // cache is full, so bar can't be inserted + EXPECT_THAT(cache.HasAsset("bar.obj"), IsNull()); +} + TEST(CacheTest, ResetAllTest) { mjCCache cache(kMaxSize); mjCAsset asset1("file1.xml", "foo.obj", "now");