szaszm commented on code in PR #1945:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1945#discussion_r2031013287
##########
libminifi/include/core/repository/VolatileContentRepository.h:
##########
@@ -17,113 +17,38 @@
#pragma once
-#include <map>
-#include <memory>
+#include <unordered_map>
#include <string>
-#include <string_view>
-
-#include "AtomicRepoEntries.h"
-#include "io/AtomicEntryStream.h"
+#include <memory>
+#include <mutex>
#include "core/ContentRepository.h"
-#include "properties/Configure.h"
-#include "core/Connectable.h"
-#include "core/logging/LoggerFactory.h"
-#include "utils/GeneralUtils.h"
-#include "VolatileRepositoryData.h"
-#include "utils/Literals.h"
namespace org::apache::nifi::minifi::core::repository {
-/**
- * Purpose: Stages content into a volatile area of memory. Note that when the
maximum number
- * of entries is consumed we will rollback a session to wait for others to be
freed.
- */
-class VolatileContentRepository : public core::ContentRepositoryImpl {
- public:
- static const char *minimal_locking;
-
- explicit VolatileContentRepository(std::string_view name =
className<VolatileContentRepository>())
- : core::ContentRepositoryImpl(name),
- repo_data_(15000, static_cast<size_t>(10_MiB * 0.75)),
- minimize_locking_(true),
- logger_(logging::LoggerFactory<VolatileContentRepository>::getLogger()) {
- }
-
- ~VolatileContentRepository() override {
- logger_->log_debug("Clearing repository");
- if (!minimize_locking_) {
- std::lock_guard<std::mutex> lock(map_mutex_);
- for (const auto &item : master_list_) {
- delete item.second;
- }
- master_list_.clear();
- }
- }
-
- uint64_t getRepositorySize() const override {
- return repo_data_.getRepositorySize();
- }
- uint64_t getMaxRepositorySize() const override {
- return repo_data_.getMaxRepositorySize();
- }
-
- uint64_t getRepositoryEntryCount() const override {
- return master_list_.size();
- }
-
- bool isFull() const override {
- return repo_data_.isFull();
- }
+class VolatileContentRepository : public ContentRepositoryImpl {
+ public:
+ explicit VolatileContentRepository(std::string_view name =
className<VolatileContentRepository>());
- /**
- * Initialize the volatile content repo
- * @param configure configuration
- */
+ uint64_t getRepositorySize() const override;
+ uint64_t getMaxRepositorySize() const override;
+ uint64_t getRepositoryEntryCount() const override;
+ bool isFull() const override;
bool initialize(const std::shared_ptr<Configure> &configure) override;
-
- /**
- * Creates writable stream.
- * @param claim resource claim
- * @return BaseStream shared pointer that represents the stream the consumer
will write to.
- */
std::shared_ptr<io::BaseStream> write(const minifi::ResourceClaim &claim,
bool append) override;
-
- /**
- * Creates readable stream.
- * @param claim resource claim
- * @return BaseStream shared pointer that represents the stream from which
the consumer will read..
- */
std::shared_ptr<io::BaseStream> read(const minifi::ResourceClaim &claim)
override;
-
bool exists(const minifi::ResourceClaim &claim) override;
+ bool close(const minifi::ResourceClaim &claim) override;
+ void clearOrphans() override;
- /**
- * Closes the claim.
- * @return whether or not the claim is associated with content stored in
volatile memory.
- */
- bool close(const minifi::ResourceClaim &claim) override {
- return remove(claim);
- }
-
- void clearOrphans() override {
- // there are no persisted orphans to delete
- }
+ ~VolatileContentRepository() override = default;
protected:
- /**
- * Closes the claim.
- * @return whether or not the claim is associated with content stored in
volatile memory.
- */
bool removeKey(const std::string& content_path) override;
private:
- VolatileRepositoryData repo_data_;
- bool minimize_locking_;
-
- // mutex and master list that represent a cache of Atomic entries. this
exists so that we don't have to walk the atomic entry list.
- // The idea is to reduce the computational complexity while keeping access
as maximally lock free as we can.
- std::mutex map_mutex_;
- std::map<ResourceClaim::Path, AtomicEntry<ResourceClaim::Path>*>
master_list_;
+ mutable std::mutex data_mtx_;
+ std::unordered_map<std::string, std::shared_ptr<std::string>> data_;
+ std::atomic<size_t> total_size_;
Review Comment:
The copilot low confidence suggestion looks valid IMO. Before C++20, the
underlying value used to be uninitialized. After that, it's
default-initialized, which in the case of integer types means uninitialized. It
doesn't seem to be initialized later either. Let's add a zero init.
https://en.cppreference.com/w/cpp/atomic/atomic/atomic
```suggestion
std::atomic<size_t> total_size_{0};
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
