diff --git a/hipfile/src/amd_detail/batch/batch.cpp b/hipfile/src/amd_detail/batch/batch.cpp index 7ff7c14f..52f1f10a 100644 --- a/hipfile/src/amd_detail/batch/batch.cpp +++ b/hipfile/src/amd_detail/batch/batch.cpp @@ -22,6 +22,72 @@ namespace hipFile { +// /* +// Factory was created with the help of an LLM +// - typename... Args - "parameter pack" of zero or more types +// - Args... - Expand the "parameter pack" +// - typename = std::enable_if_t<...> - dummy template param used as a gate +// used to check for function resolution +// Allows for trying all overloads. +// If no matching signature found at all: compiler error. +// - forward(args)... - "perfectly forward" all passed args into T's matched ctor. +// aka. expand the arg type pack and arg value pack into +// separate std::forward calls. +// */ +// template +// struct GenericFactory { + +// // Return an instance of T +// // Usage: Factory::make(...) +// template < +// typename... Args, +// typename = std::enable_if_t::value> +// > +// static T make(Args&&... args) +// { +// return T(std::forward(args)...); +// } + +// // Return a std::shared_ptr of T +// // Usage:: Factory::make_shared(...) +// template < +// typename... Args, +// typename = std::enable_if_t::value> +// > +// static std::shared_ptr make_shared(Args&&... args) +// { +// return std::make_shared(std::forward(args)...); +// } + +// // Return a std::shared_ptr of a base class of T +// // Usage: Factory::make_shared_as(...) +// template < +// typename BaseT, +// typename... Args, +// typename = std::enable_if_t< +// std::is_constructible::value && +// std::is_base_of::value +// > +// > +// static std::shared_ptr make_shared_as(Args&&... args) +// { +// return std::static_pointer_cast(std::make_shared(std::forward(args)...)); +// //return std::shared_ptr{new T{std::forward(std::forward(args)...); +// } + +// // Return a std::unique_ptr of T +// template < +// typename... Args, +// typename = std::enable_if_t::value> +// > +// static std::unique_ptr make_unique(Args&&... args) +// { +// return std::make_unique(std::forward(args)...); +// } +// }; + + BatchOperation::BatchOperation(std::unique_ptr params, std::shared_ptr _buffer, std::shared_ptr _file) : io_params{std::move(params)}, buffer{_buffer}, file{_file} @@ -99,9 +165,26 @@ BatchContext::get_capacity() const noexcept return capacity; } +//using BatchOpFactory = std::function + +// template +// void +// test_usage(Factory& fac) +// { +// (void)fac; +// } + +// void +// test_caller() +// { +// Factory fac; +// test_usage(fac); +// } + void -BatchContext::submit_operations(const hipFileIOParams_t *params, unsigned num_params) +BatchContext::submit_operations(const hipFileIOParams_t *params, unsigned num_params, IFactory factory) { + (void)factory; std::unique_lock _ulock{context_mutex}; // Check num_params first before doing anything else @@ -113,7 +196,7 @@ BatchContext::submit_operations(const hipFileIOParams_t *params, unsigned num_pa throw std::invalid_argument(msg.str()); } - std::vector> pending_ops{}; + std::vector> pending_ops{}; // It would be more performant to be able to perform multiple lookups // rather than waiting to lock the DriverState lock for each lookup. @@ -124,7 +207,8 @@ BatchContext::submit_operations(const hipFileIOParams_t *params, unsigned num_pa // file flags. auto [_file, _buffer] = Context::get()->getFileAndBuffer( param_copy->fh, param_copy->u.batch.devPtr_base, param_copy->u.batch.size, 0); - auto op = std::make_shared(std::move(param_copy), _buffer, _file); + auto op = std::shared_ptr{new BatchOperation{std::move(param_copy), _buffer, _file}}; + pending_ops.push_back(op); } diff --git a/hipfile/src/amd_detail/batch/batch.h b/hipfile/src/amd_detail/batch/batch.h index f8372a17..c75610f2 100644 --- a/hipfile/src/amd_detail/batch/batch.h +++ b/hipfile/src/amd_detail/batch/batch.h @@ -22,14 +22,94 @@ class IFile; namespace hipFile { +/* + * Intentionally empty interface + */ +template +class IFactory { +}; + +/* + Factory was created with the help of an LLM + - typename... Args - "parameter pack" of zero or more types + - Args... - Expand the "parameter pack" + - typename = std::enable_if_t<...> - dummy template param used as a gate + used to check for function resolution + Allows for trying all overloads. + If no matching signature found at all: compiler error. + - forward(args)... - "perfectly forward" all passed args into T's matched ctor. + aka. expand the arg type pack and arg value pack into + separate std::forward calls. +*/ +template < + typename Interface, + typename Implementation +> +class GenericFactory { +public: + // Return an instance of Implementation + // Usage: Factory::make(...) + template < + typename... Args, + typename = std::enable_if_t::value> + > + static Implementation make(Args&&... args) + { + return Implementation(std::forward(args)...); + } + + // Return a std::shared_ptr to an Implementation instance + // Usage:: Factory::make_shared(...) + template < + typename... Args, + typename = std::enable_if_t::value> + > + static std::shared_ptr make_shared(Args&&... args) + { + return std::static_pointer_cast(std::make_shared(std::forward(args)...)); + } + + // // Return a std::shared_ptr to an arbitrary + // // Usage: Factory::make_shared_as(...) + // template < + // typename BaseT, + // typename... Args, + // typename = std::enable_if_t< + // std::is_constructible::value && + // std::is_base_of::value + // > + // > + // static std::shared_ptr make_shared_as(Args&&... args) + // { + // return std::static_pointer_cast(std::make_shared(std::forward(args)...)); + // //return std::shared_ptr{new T{std::forward(std::forward(args)...); + // } + + // // Return a std::unique_ptr of T + // template < + // typename... Args, + // typename = std::enable_if_t::value> + // > + // static std::unique_ptr make_unique(Args&&... args) + // { + // return std::make_unique(std::forward(args)...); + // } +}; + struct InvalidBatchHandle : public std::invalid_argument { InvalidBatchHandle() : std::invalid_argument{"Invalid batch handle"} { } }; +class IBatchOperation { +public: + virtual ~IBatchOperation() = default; +}; + /// @brief Represents a single IO Request -class BatchOperation { +class BatchOperation : public IBatchOperation { public: /// @brief Create an operation to handle and track an IO request. /// @param [in] params IO parameters @@ -50,13 +130,21 @@ class BatchOperation { const std::shared_ptr file; }; +template < + typename Implementation, + typename Interface = IBatchOperation +> +class BatchOperationFactory : public IFactory, public GenericFactory +{ +}; + class IBatchContext { public: static constexpr unsigned MAX_SIZE = 128; virtual ~IBatchContext() = default; virtual unsigned get_capacity() const noexcept = 0; - virtual void submit_operations(const hipFileIOParams_t *params, unsigned num_params) = 0; + virtual void submit_operations(const hipFileIOParams_t *params, unsigned num_params, IFactory factory = BatchOperationFactory{}) = 0; }; class BatchContext : public IBatchContext { @@ -76,7 +164,7 @@ class BatchContext : public IBatchContext { /// @note This is an All or None operation. If one submitted operation is not valid, no operations /// will be submitted. /// - void submit_operations(const hipFileIOParams_t *params, const unsigned num_params) override; + void submit_operations(const hipFileIOParams_t *params, const unsigned num_params, IFactory factory) override; private: const unsigned capacity; @@ -89,7 +177,7 @@ class BatchContext : public IBatchContext { /// but is not yet complete or completed but not yet retrieved by the /// application. /// shared_ptr as it may need to be passed to a backend. - std::unordered_set> outstanding_ops; + std::unordered_set> outstanding_ops; BatchContext(unsigned capacity); @@ -131,4 +219,6 @@ class BatchContextMap { mutable std::shared_mutex batch_mutex; }; +static_assert(!std::is_abstract>::value, "Not concrete"); + } diff --git a/hipfile/test/amd_detail/mbatch.h b/hipfile/test/amd_detail/mbatch.h index c59ba908..a4e799db 100644 --- a/hipfile/test/amd_detail/mbatch.h +++ b/hipfile/test/amd_detail/mbatch.h @@ -15,10 +15,13 @@ namespace hipFile { +class MBatchOperation : public IBatchOperation { +}; + class MBatchContext : public IBatchContext { public: MOCK_METHOD(unsigned, get_capacity, (), (const, noexcept, override)); - MOCK_METHOD(void, submit_operations, (const hipFileIOParams_t *params, const unsigned num_params), + MOCK_METHOD(void, submit_operations, (const hipFileIOParams_t *params, const unsigned num_params, IFactory factory), (override)); };