Incremental build optimizations part 2 by atobiszei · Pull Request #4147 · openvinotoolkit/model_server

atobiszei · 2026-04-17T08:41:50Z

Continue reducing transitive dependencies and preprocessed translation unit (TU) sizes across the OVMS codebase. This PR focuses on three complementary tracks:

Eliminate openvino/openvino.hpp umbrella from frequently-included headers - replacing with narrow, specific OpenVINO includes across ~60 files.
Split monolithic DAG TUs per-frontend** - so changing one frontend (TFS/KFS/CAPI) doesn't recompile the others.
Decouple DAG subsystem from ModelManager internals - DAG code no longer depends on modelinstance.hpp or modelinstanceunloadguard.hpp.
Added RapidJSON port wrappers (rapidjson_error.hpp, rapidjson_istreamwrapper.hpp) matching the existing rapidjson_stringbuffer.hpp/rapidjson_writer.hpp pattern - wrapping third-party includes with #pragma GCC diagnostic ignored
Extract model, modelinstance, modelmanager, servablemanagermodule as Standalone Bazel Targets

Expanded ModelInstanceProvider interface (from Part 1) with three new methods:

getModelInputsInfo(name, version, tensor_map_t&) - returns model input tensor metadata, internally handling ModelInstanceUnloadGuard lifecycle
getModelOutputsInfo(name, version, tensor_map_t&) - same for outputs
hasAutoModelParameters(name, version, batchAuto&, shapeAuto&) - queries auto batch/shape without exposing ModelInstance

New DagResourceManager interface (src/dags/dag_resource_manager.hpp):

addResourceToCleaner(shared_ptr<CNLIMWrapper>) - abstract interface for custom node library cleanup
Implemented by ModelManager

Extracted pipeline_config_parser (src/dags/pipeline_config_parser.cpp/.hpp):

Pipeline JSON config parsing moved from modelmanager.cpp into its own file
Depends only on interfaces (ModelInstanceProvider, ServableNameChecker, DagResourceManager) - not on ModelManager

Key results (measured on //src:ovms target, --config=mp_on_py_on):

Primary KPI view (non-test files only):

Metric	Before	After	Change
Dependency files measured	258	261	+3
Average total deps/file	49.0	42.8	-12.6%
Average internal deps/file	33.5	29.7	-11.5%
Files in >=201 deps bucket	24	14	-10
TU files measured	1,131	1,134	+3
Total preprocessed lines	61,250,065	60,489,996	-1.2%
Total preprocessed size (MB)	2208.3	2177.8	-1.4%
Average PP lines/file	54,156	53,342	-1.5%
Files in >=300001 PP-lines bucket	2	2	0

Scope comparison (dependencies):

Scope	Files	Avg total deps/file	Avg internal deps/file
All files	314 -> 317	64.1 -> 60.3 (-5.8%)	44.8 -> 43.0 (-3.9%)
Non-test files	258 -> 261	49.0 -> 42.8 (-12.6%)	33.5 -> 29.7 (-11.5%)
Test files	56 -> 56	133.5 -> 142.1 (+6.5%)	96.5 -> 105.2 (+8.9%)

Dependency buckets (non-test):

Bucket	Before	After	Change
0-5 deps	38	38	0
6-10 deps	35	35	0
11-20 deps	66	66	0
21-50 deps	59	62	+3
51-100 deps	36	41	+5
101-200 deps	0	5	+5
>=201 deps	24	14	-10

PP-line buckets (non-test TU view):

Bucket	Before	After	Change
0-10000	225	225	0
10001-25000	143	143	0
25001-50000	195	195	0
50001-100000	471	480	+9
100001-200000	77	73	-4
200001-300000	18	16	-2
>=300001	2	2	0

Top individual TU reductions:

File	Before	After	Reduction
pipelinedefinition.cpp	195K	90K	-53.7%
pipeline_factory.cpp	178K	83K	-53.4%
cleaner_utils.cpp	95K	70K	-26.9%
modelinstanceunloadguard.cpp	107K	87K	-18.2%
capi_backend_impls.cpp	134K	115K	-14.3%
kfs_backend_impl.cpp	178K	159K	-10.7%
model_service.cpp	157K	140K	-11.3%
tfs_backend_impl.cpp	286K	269K	-5.9%
get_model_metadata_impl.cpp	287K	271K	-5.4%
capi.cpp	297K	281K	-5.2%

Diff Breakdown

106 files changed, +1677/-955. Most additions are mechanical code moves, new BUILD targets, and include narrowing — not new logic. Rough split of the added lines:

Category	Lines	Notes
BUILD target definitions	~320	New Bazel `cc_library` targets (`dag_{capi,kfs,tfs}`, `pipeline_config_parser`, `servablemanagermodule`, `model`, `statefulmodelinstance`, etc.)
Template `_impl.hpp` headers	~380	Code moved out of monolithic `.cpp` files (`pipelinedefinition_create_impl.hpp`, `pipeline_factory_create_impl.hpp`, `exit_node_impl.hpp`, `exitnodesession_impl.hpp`, `entry_node_impl.hpp`)
`pipeline_config_parser.cpp/.hpp`	~260	Extracted from `modelmanager.cpp`
Per-frontend `.cpp` stubs	~170	`dag_{capi,kfs,tfs}.cpp` — includes + explicit template instantiations
Copyright headers on new files	~130	16-line Apache 2.0 header per new file
Port wrappers (RapidJSON)	~60	`src/port/rapidjson_{document,error,istreamwrapper}.hpp`
`#include` narrowing	remainder	Replacing `openvino/openvino.hpp` with specific OV headers across ~60 files
Truly new logic	~75	Interface signatures (`ModelInstanceProvider` additions, `DagResourceManager`) + `ModelManager` implementations

…apidjson port wrappers - Replace openvino/openvino.hpp with narrow openvino/core/type/element_type.hpp in tensorinfo.hpp and precision.hpp - Add explicit openvino/core/shape.hpp and openvino/runtime/tensor.hpp to kfs_utils.hpp (was getting them transitively through umbrella) - Add src/port/rapidjson_istreamwrapper.hpp and src/port/rapidjson_error.hpp port wrappers with MSVC C6313 pragma guards - Replace inline pragma guards in embeddings_servable.hpp and rerank_servable.hpp with port wrapper includes

- Replace openvino/openvino.hpp with narrow openvino/core/any.hpp (for ov::AnyMap) - Remove openvino GPU headers from header, forward-declare ClContext/VAContext/RemoteTensor - Add CL/cl.h for cl_context member (7K vs 122K lines for OV OCL wrapper) - Forward-declare ov::Core, Model, CompiledModel, InferRequest, PartialShape - Move getBatchSize() inline body to .cpp (uses ov::get_batch) - Add openvino/core/layout.hpp to .cpp for ov::get_batch

- Remove rapidjson/document.h from modelmanager.hpp via ConfigLoader friend struct All 6 rapidjson-signature methods moved to ConfigLoader static methods in .cpp Saves ~2.5K PP lines x ~290 includers (~725K total) - Remove ovinferrequestsqueue.hpp include from modelinstance.hpp Forward-declare OVInferRequestsQueue, move getInferRequestsQueue() body to .cpp - Narrow ovinferrequestsqueue.hpp: openvino.hpp -> infer_request.hpp + compiled_model.hpp - Change MetricConfig member to unique_ptr (fwd-decl in header, include in .cpp) - Remove dead setMetricConfig() method - Fix cascading: add explicit includes in dlnodesession.cpp, embeddings_servable.cpp, http_rest_api_handler.cpp, test files

Replace openvino/openvino.hpp with specific narrow includes in all non-test headers. Each header now includes only the OV types it uses: - ov::Tensor -> openvino/runtime/tensor.hpp - ov::InferRequest -> openvino/runtime/infer_request.hpp - ov::Shape/PartialShape -> openvino/core/shape.hpp etc. - ov::element::Type -> openvino/core/type/element_type.hpp - ov::Layout -> openvino/core/layout.hpp - ov::Any -> openvino/core/any.hpp - ov::Core -> openvino/runtime/core.hpp 2 dead includes removed (exitnodesession.hpp, nodeinputhandler.hpp). Cascading fixes in 3 .cpp files + 17 test files that lost transitive OV types.

Create //src:statefulmodelinstance and //src:model cc_library targets, removing model.cpp/hpp and statefulmodelinstance.cpp/hpp from ovms_lib srcs. Both added as ovms_lib deps. Also narrow statefulmodelinstance.cpp: replace openvino/openvino.hpp with openvino/core/except.hpp + openvino/runtime/infer_request.hpp.

…ttern Split monolithic entry_node.cpp and exit_node.cpp (which included all 3 frontend headers: KFS, TFS, CAPI) into: - entry_node_impl.hpp / exit_node_impl.hpp: frontend-free template bodies - entry_node_{capi,kfs,tfs}.cpp / exit_node_{capi,kfs,tfs}.cpp: per-frontend explicit template instantiations with only their own frontend includes Uses C++ two-phase template lookup: template bodies in _impl.hpp reference per-frontend overloads (isNativeFileFormatUsed, getRequestServableName, serializePredictResponse) that are resolved at instantiation time, not at template definition time. Each per-frontend .cpp includes its frontend headers before _impl.hpp so overloads are visible at instantiation. Moved OutputGetter<TensorMap> and InputSink<TensorWithSourceMap> full specializations to _impl.hpp as inline (not frontend-specific). All 5 build configs pass. 4020/4020 tests pass.

…from gatherexitnodeinputhandler.hpp - Remove 4 frontend includes (capi_utils, capi_dag_utils, kfs_utils, tfs_utils) from gatherexitnodeinputhandler.hpp, add logging.hpp for SPDLOG_ERROR - Split exitnodesession.cpp into per-frontend TUs: exitnodesession_capi.cpp, exitnodesession_kfs.cpp, exitnodesession_tfs.cpp + exitnodesession_impl.hpp with template bodies - Add explicit frontend includes to gather_node_test.cpp (previously transitive) - Update BUILD exports_files and ovms_lib srcs - Copyright year fix on per-frontend entry/exit node files - All 5 build configs pass, 4020/4020 tests pass

…gets - Create 12 new cc_library targets (header + 3 per-frontend for each): exitnodesession, exitnodesession_{capi,kfs,tfs} exit_node, exit_node_{capi,kfs,tfs} entry_node, entry_node_{capi,kfs,tfs} - Each per-frontend target depends only on its own frontend deps (e.g. entry_node_kfs depends on kfs_deserialization, not tfs/capi) - Remove entry/exit/exitnodesession files from ovms_lib srcs, add the 12 targets to ovms_lib deps - Remove gatherexitnodeinputhandler.hpp from ovms_lib srcs (now only in exitnodesession target hdrs) - All 5 build configs pass, 4020/4020 tests pass

- Add getModelInputsInfo/getModelOutputsInfo/hasAutoModelParameters to ModelInstanceProvider interface - Remove modelinstance/modelinstanceunloadguard deps from pipelinedefinition - Merge modelmanager_h into modelmanager BUILD target - Extract processPipelineConfig to src/dags/pipeline_config_parser - Merge fetchDependantModelMetadata+retrieveDependantMetadata into fetchDependantMetadata - Remove unused dags deps (entry_node, exit_node, node_library, pipeline) from modelmanager - Extract servablemanagermodule as separate BUILD target from ovms_lib

# Conflicts: # src/test/ensemble_tests.cpp

…cessor specializations

…TFS,TFS> to emit vtable+specializations in tfs_request_utils.obj

…<> specialization)

Copilot

Pull request overview

This PR continues incremental build-time optimizations by narrowing OpenVINO includes, further decoupling the DAG/pipeline subsystem from ModelManager internals, and splitting DAG template instantiations per frontend to reduce transitive dependencies and recompilation scope.

Changes:

Replaced openvino/openvino.hpp umbrella includes with narrower OpenVINO headers across many headers/tests.
Refactored DAG/pipeline code to depend on new interfaces (ModelInstanceProvider additions, DagResourceManager) and extracted pipeline JSON parsing into pipeline_config_parser.
Split previously monolithic DAG template instantiations into per-frontend compilation units (dag_{capi,kfs,tfs}.cpp) and introduced new Bazel targets to isolate build dependencies.

Reviewed changes

Copilot reviewed 108 out of 108 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/vaapitensorfactory.hpp	Narrow OpenVINO includes for VAAPI tensor factory.
src/tfs_frontend/tfs_request_utils.cpp	Adds explicit template instantiation for stateful request processing (TFS).
src/tfs_frontend/serialization.hpp	Narrow OpenVINO includes for TFS serialization.
src/tfs_frontend/deserialization.hpp	Narrow OpenVINO includes for TFS deserialization.
src/test/stress_test_utils.hpp	Adjust test includes for metrics config dependency changes.
src/test/stateful_test_utils.hpp	Narrow OpenVINO includes used by stateful tests.
src/test/stateful_modelinstance_test.cpp	Add OpenVINO Core include for tests.
src/test/serialization_tests.cpp	Add OpenVINO Core include for tests.
src/test/predict_validation_test.cpp	Add OpenVINO Core include for tests.
src/test/ovinferrequestqueue_test.cpp	Add OpenVINO Core include for tests.
src/test/ov_utils_test.cpp	Add OpenVINO Core include for tests.
src/test/openvino_remote_tensors_tests.cpp	Adjust OpenVINO includes in remote tensor tests.
src/test/modelmanager_test.cpp	Add OpenVINO Core include for tests.
src/test/modelinstance_test.cpp	Add OpenVINO Core include for tests.
src/test/model_test.cpp	Add OpenVINO Core include for tests.
src/test/model_cache_test.cpp	Add OpenVINO Core include for tests.
src/test/mediapipeflow_test.cpp	Add OpenVINO Core include for tests.
src/test/kfs_metadata_test.cpp	Add OpenVINO Core include for tests.
src/test/get_model_metadata_response_test.cpp	Add OpenVINO Core include for tests.
src/test/gather_node_test.cpp	Update test includes after DAG/frontend decoupling.
src/test/ensemble_tests.cpp	Update pipeline factory/definition calls for expanded interfaces; add metrics config include.
src/test/ensemble_metadata_test.cpp	Update validation calls to new `validate(provider, nameChecker, resourceMgr)` signature.
src/test/ensemble_mapping_config_tests.cpp	Update pipeline factory/definition calls to expanded signatures.
src/test/ensemble_flow_custom_node_tests.cpp	Update factory/definition calls and includes; add OpenVINO Core include.
src/test/deserialization_tests.cpp	Add OpenVINO Core include for tests.
src/test/custom_node_output_allocator_test.cpp	Narrow OpenVINO includes for allocator tests.
src/test/capi_predict_validation_test.cpp	Add OpenVINO Core include for tests.
src/test/c_api_tests.cpp	Add OpenVINO Core include for tests.
src/tensorinfo.hpp	Narrow OpenVINO includes in tensor metadata types.
src/tensor_utils.hpp	Narrow OpenVINO includes for tensor utilities.
src/statefulmodelinstance.cpp	Narrow OpenVINO includes used by stateful model instance.
src/sidepacket_servable.cpp	Adds OpenVINO Core include for sidepacket servable implementation.
src/shape.hpp	Narrow OpenVINO includes for shape utilities.
src/serialization_common.hpp	Narrow OpenVINO includes for shared serialization helpers.
src/serialization_common.cpp	Adds missing OpenVINO InferRequest include for implementation.
src/sequence.hpp	Narrow OpenVINO includes for sequence/state handling.
src/resources_cleaner.hpp	New interface to decouple resource cleanup from `ModelManager`.
src/rerank/rerank_utils.hpp	Narrow OpenVINO includes in rerank utilities.
src/rerank/rerank_servable.hpp	Switch to RapidJSON port wrappers to reduce warning noise / transitive deps.
src/rerank/BUILD	Add deps for new RapidJSON wrappers.
src/requestprocessor.hpp	Narrow OpenVINO includes for request processor interface.
src/regularovtensorfactory.hpp	Narrow OpenVINO includes for regular tensor factory.
src/prediction_service_utils.hpp	Refactor/trim header contents to reduce transitive frontend deps.
src/precision_configuration.hpp	Narrow OpenVINO includes for precision config.
src/precision.hpp	Narrow OpenVINO includes for precision types.
src/port/rapidjson_istreamwrapper.hpp	New RapidJSON port wrapper header.
src/port/rapidjson_error.hpp	New RapidJSON port wrapper header.
src/port/BUILD	Adds Bazel targets for new RapidJSON wrapper headers.
src/ovinferrequestsqueue.hpp	Narrow OpenVINO includes for infer request queue.
src/ov_utils.hpp	Replace umbrella include with targeted OpenVINO headers.
src/outputkeeper.hpp	Narrow OpenVINO includes for output keeper.
src/opencltensorfactory.hpp	Narrow OpenVINO includes for OpenCL tensor factory.
src/modelmanager.hpp	Refactor ModelManager interfaces/ownership (metrics config ptr, DAG resource mgmt, provider interface expansion).
src/modelmanager.cpp	Extract pipeline config parsing; implement new `ModelInstanceProvider` methods; update metrics config ownership.
src/modelinstance.hpp	Reduce transitive deps via forward declarations; move method bodies to .cpp.
src/modelinstance.cpp	Add missing includes after header slimming; provide moved method definitions.
src/model_instance_provider.hpp	Expand provider interface (model lookup, subscriptions, metadata queries, auto-parameter query).
src/llm/text_utils.hpp	Narrow OpenVINO includes for LLM token utilities.
src/layout.hpp	Narrow OpenVINO includes for layout utilities.
src/kfs_frontend/serialization.hpp	Narrow OpenVINO includes for KFS serialization.
src/kfs_frontend/kfs_utils.hpp	Add needed OpenVINO tensor/shape includes for KFS utils.
src/kfs_frontend/deserialization.hpp	Narrow OpenVINO includes for KFS deserialization.
src/json_parser.hpp	Narrow OpenVINO includes for JSON parser utilities.
src/itensorfactory.hpp	Narrow OpenVINO includes for tensor factory interface.
src/image_gen/imagegenutils.hpp	Narrow OpenVINO includes in image generation utilities.
src/image_gen/imagegenpipelineargs.hpp	Narrow OpenVINO includes for image gen pipeline args.
src/http_rest_api_handler.cpp	Adjust includes after moving request utils / metrics config ownership changes.
src/embeddings/embeddings_servable.hpp	Switch to RapidJSON port wrappers.
src/embeddings/embeddings_servable.cpp	Narrow OpenVINO includes / add required headers for embeddings servable implementation.
src/embeddings/BUILD	Add deps for RapidJSON wrappers.
src/deserialization_main.hpp	Narrow OpenVINO includes for main deserialization header.
src/deserialization_common.hpp	Narrow OpenVINO includes for shared deserialization code.
src/deserialization_common.cpp	Add missing OpenVINO InferRequest include for implementation.
src/dags/tensormap.hpp	Narrow OpenVINO includes for DAG tensor map.
src/dags/pipelinedefinition_create_impl.hpp	New template implementation header to decouple/split pipeline create logic.
src/dags/pipelinedefinition.hpp	Update DAG definition interfaces to depend on providers/checkers/resource mgr.
src/dags/pipelinedefinition.cpp	Refactor to use new provider/resource interfaces and remove ModelManager dependency.
src/dags/pipeline_factory_create_impl.hpp	New template implementation header to decouple/split pipeline creation.
src/dags/pipeline_factory.hpp	Update factory interfaces to accept provider/nameChecker/resourceMgr and optional metrics pointers.
src/dags/pipeline_factory.cpp	Implement updated factory interfaces; remove frontend-specific instantiations from this TU.
src/dags/pipeline_config_parser.hpp	New extracted pipeline JSON config parser API.
src/dags/pipeline_config_parser.cpp	New extracted pipeline JSON config parsing implementation.
src/dags/nodeinputhandler.hpp	Remove umbrella OpenVINO include from node input handler.
src/dags/node.hpp	Narrow OpenVINO includes for base DAG node.
src/dags/gathernodeinputhandler.hpp	Narrow OpenVINO includes used by gather handler.
src/dags/gatherexitnodeinputhandler.hpp	Remove frontend utils includes; narrow OpenVINO includes and adjust dependencies.
src/dags/exitnodesession_impl.hpp	Move exit node session template definitions/instantiations toward per-frontend TUs.
src/dags/exitnodesession.hpp	Remove umbrella OpenVINO include from exit node session header.
src/dags/exit_node_impl.hpp	Restructure exit node templates and move specializations/instantiations toward per-frontend TUs.
src/dags/entry_node_impl.hpp	Restructure entry node templates and move specializations/instantiations toward per-frontend TUs.
src/dags/entry_node.hpp	Narrow OpenVINO includes for entry node.
src/dags/dlnodesession.hpp	Narrow OpenVINO includes for DL node sessions.
src/dags/dlnodesession.cpp	Add missing includes after header slimming.
src/dags/dl_node.hpp	Narrow OpenVINO includes for DL node.
src/dags/dag_tfs.cpp	New per-frontend TFS DAG instantiation TU.
src/dags/dag_resource_manager.hpp	New interface for DAG resource cleanup plumbing.
src/dags/dag_kfs.cpp	New per-frontend KFS DAG instantiation TU.
src/dags/dag_capi.cpp	New per-frontend CAPI DAG instantiation TU.
src/dags/customnodesession.hpp	Narrow OpenVINO includes for custom node session.
src/dags/BUILD	Add new Bazel targets for extracted DAG components and per-frontend instantiation TUs.
src/color_format_configuration.hpp	Narrow OpenVINO includes for preprocess configuration.
src/cleaner_utils.hpp	Decouple cleaner utilities from ModelManager via ResourcesCleaner interface.
src/cleaner_utils.cpp	Update implementation to use ResourcesCleaner interface.
src/capi_frontend/serialization.hpp	Narrow OpenVINO includes for CAPI serialization.
src/capi_frontend/deserialization.hpp	Narrow OpenVINO includes for CAPI deserialization.
src/capi_frontend/capi_dag_utils.hpp	Narrow OpenVINO includes for CAPI DAG utils.
src/BUILD	Extract additional Bazel targets and update dependencies to reflect refactors.
spelling-whitelist.txt	Update whitelisted line reference after header edits.

Copilot · 2026-04-23T10:48:32Z

+const std::string ENTRY_NODE_NAME = "request";
+const std::string EXIT_NODE_NAME = "response";
+const std::string DEFAULT_PIPELINE_NAME = "";


ENTRY_NODE_NAME/EXIT_NODE_NAME/DEFAULT_PIPELINE_NAME are defined only in dag_capi.cpp, while entry_node.hpp/exit_node.hpp expose them as extern and they are used by non-CAPI code (e.g., pipeline_config_parser.cpp). This makes DAG components implicitly depend on linking the CAPI frontend; builds/targets that link DAG + KFS/TFS without dag_capi will get undefined symbols. Consider moving these constants to a frontend-agnostic TU (e.g., dags/constants.cpp) or making them inline constexpr in a shared header used by all frontends, and have dag_capi only contain CAPI-specific instantiations.

Suggested change

const std::string ENTRY_NODE_NAME = "request";

const std::string EXIT_NODE_NAME = "response";

const std::string DEFAULT_PIPELINE_NAME = "";

// Frontend-agnostic DAG constants are declared in shared headers and must be

// defined in a frontend-agnostic translation unit, not in this CAPI-specific

// file.

Copilot · 2026-04-23T10:48:32Z

 std::optional<Dimension> getRequestBatchSize(const RequestType* request, const size_t batchSizeIndex);
 template <typename RequestType>
 std::map<std::string, shape_t> getRequestShapes(const RequestType* request);
-std::optional<Dimension> getRequestBatchSize(const ::KFSRequest* request, const size_t batchSizeIndex);
-template <typename RequestType>
-std::map<std::string, shape_t> getRequestShapes(const ::KFSRequest* request);
-
-std::optional<Dimension> getRequestBatchSize(const tensorflow::serving::PredictRequest* request, const size_t batchSizeIndex);
-std::map<std::string, shape_t> getRequestShapes(const tensorflow::serving::PredictRequest* request);
-
-template <>
-class RequestTensorExtractor<tensorflow::serving::PredictRequest, tensorflow::TensorProto, ExtractChoice::EXTRACT_OUTPUT> {
-public:
-    static Status extract(const tensorflow::serving::PredictRequest& request, const std::string& name, const tensorflow::TensorProto** tensor, size_t* bufferId) {
-        return StatusCode::NOT_IMPLEMENTED;
-    }
-};
-
-template <>
-class RequestTensorExtractor<tensorflow::serving::PredictRequest, tensorflow::TensorProto, ExtractChoice::EXTRACT_INPUT> {
-public:
-    static Status extract(const tensorflow::serving::PredictRequest& request, const std::string& name, const tensorflow::TensorProto** tensor, size_t* bufferId) {
-        if (bufferId == nullptr) {
-            return StatusCode::INTERNAL_ERROR;
-        }
-        auto it = request.inputs().find(name);
-        if (it == request.inputs().end()) {
-            return StatusCode::NONEXISTENT_TENSOR;
-        }
-        *tensor = &it->second;
-        return StatusCode::OK;
-    }
-};
-
-template <>
-class RequestTensorExtractor<KFSRequest, KFSTensorInputProto, ExtractChoice::EXTRACT_OUTPUT> {
-public:
-    static Status extract(const KFSRequest& request, const std::string& name, const KFSTensorInputProto** tensor, size_t* bufferId) {
-        return StatusCode::NOT_IMPLEMENTED;
-    }
-};
-
-template <>
-class RequestTensorExtractor<KFSRequest, KFSTensorInputProto, ExtractChoice::EXTRACT_INPUT> {
-public:
-    static Status extract(const KFSRequest& request, const std::string& name, const KFSTensorInputProto** tensor, size_t* bufferId) {
-        if (bufferId == nullptr) {
-            return StatusCode::INTERNAL_ERROR;
-        }
-        size_t id = 0;
-        auto it = request.inputs().begin();
-        while (it != request.inputs().end()) {
-            if (it->name() == name) {
-                break;
-            }
-            ++it;
-            ++id;
-        }
-        if (it == request.inputs().end()) {
-            return StatusCode::NONEXISTENT_TENSOR;
-        }
-        *bufferId = id;
-        *tensor = &(*it);
-        return StatusCode::OK;
-    }
-};

-/**
- * This is specific check required for passing KFS API related info
- * which informs how response should be formatted. Therefore return value should not have an impact for
- * any other frontend.
- */
-bool useSharedOutputContentFn(const tensorflow::serving::PredictRequest* request);
-bool useSharedOutputContentFn(const ::KFSRequest* request);
 bool useSharedOutputContentFn(const InferenceRequest* request);
 }  // namespace ovms


prediction_service_utils.hpp declares useSharedOutputContentFn(const InferenceRequest*) but no longer includes the InferenceRequest definition, and there is no visible forward declaration in the header section shown in this diff. This will fail to compile if no other included header provides the declaration. Add a forward declaration (class InferenceRequest;) in namespace ovms or include the appropriate header that defines it.

class InferenceRequest; is forward-declared right there on line 34.

…eaders

rasapala · 2026-04-27T16:23:11Z

        }
    }
-    status = pipelineFactory->revalidatePipelines(*this);
+    status = pipelineFactory->revalidatePipelines(*this, *this, *this);


Please explain the logic behind this and add comments.

We need to follow Interface Segregation Principle here, to be able to split dependencies.

rasapala · 2026-04-27T16:25:07Z

    return it != models.end() ? it->second : nullptr;
 }

+bool ModelManager::subscribeToModel(const std::string& name, model_version_t version, NotifyReceiver& receiver) {


Add comments what does subscription mean.

rasapala · 2026-04-27T16:25:26Z

+    return true;
+}
+
+void ModelManager::unsubscribeFromModel(const std::string& name, model_version_t version, NotifyReceiver& receiver) {


Add comments what does unsubscribe mean.

rasapala · 2026-04-27T16:30:10Z

+
+struct CNLIMWrapper;
+
+class DagResourceManager {


Add description what it is used for.

rasapala · 2026-04-27T16:32:22Z

+#pragma once
+
+namespace ovms {
+class ResourcesCleaner {


Please explain purpose and add class description.

rasapala · 2026-04-27T16:33:37Z

+class ServableNameChecker;
+class Status;
+
+Status loadPipelinesConfig(


Add high level purpose description.

rasapala · 2026-04-27T16:36:18Z

@@ -42,7 +46,11 @@ class PipelineFactory {
    Status createDefinition(const std::string& pipelineName,


Add parameters description.

rasapala · 2026-04-27T16:36:56Z


    // createDefinition fails due to initialization failure
-    ASSERT_EQ(factory.createDefinition("my_new_pipeline", info, connections, manager), StatusCode::NODE_LIBRARY_INITIALIZE_FAILED);
+    ASSERT_EQ(factory.createDefinition("my_new_pipeline", info, connections, manager, manager, manager), StatusCode::NODE_LIBRARY_INITIALIZE_FAILED);


What is the purpose of 3 manager arguments ? Make it null if optional maybe ?

We are separating interaces so that user of each part of interface is not exposed to entire modelmanager, and does not have to depend on it.
Here we are passing 3 implementations of different interfaces.

dkalinowski · 2026-04-28T10:57:25Z

- */
-bool useSharedOutputContentFn(const tensorflow::serving::PredictRequest* request);
-bool useSharedOutputContentFn(const ::KFSRequest* request);
 bool useSharedOutputContentFn(const InferenceRequest* request);


Why is C-API version of this function still here?

dkalinowski · 2026-04-28T10:57:53Z

@@ -37,76 +29,6 @@ template <typename RequestType>
 std::optional<Dimension> getRequestBatchSize(const RequestType* request, const size_t batchSizeIndex);


couldnt these 2 functions be moved elsewhere as well? if so, this header could be removed

These two template declarations are the shared interface used by inference_executor.hpp in generic (templated) deserialization/reshaping code. Each frontend provides the concrete implementation in its own *_request_utils.cpp. The header can't be removed because inference_executor.hpp needs to see the template declarations without depending on any specific frontend.

atobiszei added 3 commits April 16, 2026 09:52

atobiszei commented Apr 17, 2026

View reviewed changes

Comment thread src/modelmanager.cpp Outdated

atobiszei added 6 commits April 17, 2026 11:58

atobiszei force-pushed the atobisze_build2_p1 branch from 0f5fddd to dcb11c5 Compare April 21, 2026 13:04

atobiszei added 2 commits April 21, 2026 16:35

Add missing resources_cleaner.hpp (fix CI build)

d4d306e

Merge remote-tracking branch 'origin/main' into atobisze_build2_p1

06a8a6e

# Conflicts: # src/test/ensemble_tests.cpp

atobiszei commented Apr 22, 2026

View reviewed changes

Comment thread src/dags/pipeline_factory.cpp Outdated

atobiszei commented Apr 22, 2026

View reviewed changes

Comment thread src/BUILD Outdated

atobiszei force-pushed the atobisze_build2_p1 branch from d674ee5 to 56d6b7c Compare April 22, 2026 10:13

Self-review

39236d6

atobiszei force-pushed the atobisze_build2_p1 branch 2 times, most recently from 56d6b7c to 39236d6 Compare April 23, 2026 06:53

Windows: mark tfs_request_utils alwayslink for TFS StatefulRequestPro…

542d77d

…cessor specializations

atobiszei force-pushed the atobisze_build2_p1 branch from b0f0613 to 542d77d Compare April 23, 2026 08:00

atobiszei added 2 commits April 23, 2026 11:30

Windows: explicit template instantiation of StatefulRequestProcessor<…

e4e1ba1

…TFS,TFS> to emit vtable+specializations in tfs_request_utils.obj

Fix: move explicit template instantiation to end of TU (after release…

a9a8e14

…<> specialization)

atobiszei marked this pull request as ready for review April 23, 2026 10:43

Copilot AI review requested due to automatic review settings April 23, 2026 10:43

Copilot started reviewing on behalf of atobiszei April 23, 2026 10:43 View session

Copilot AI reviewed Apr 23, 2026

View reviewed changes

Move ENTRY/EXIT_NODE_NAME, DEFAULT_PIPELINE_NAME to inline const in h…

bfe4720

…eaders

atobiszei requested review from dkalinowski and rasapala April 23, 2026 14:30

rasapala reviewed Apr 27, 2026

View reviewed changes

rasapala requested changes Apr 27, 2026

View reviewed changes

dkalinowski reviewed Apr 28, 2026

View reviewed changes

Review fixes

bd8b7ee

dkalinowski approved these changes Apr 28, 2026

View reviewed changes

atobiszei requested a review from rasapala April 28, 2026 12:54

		@@ -42,7 +46,11 @@ class PipelineFactory {
		Status createDefinition(const std::string& pipelineName,

		@@ -37,76 +29,6 @@ template <typename RequestType>
		std::optional<Dimension> getRequestBatchSize(const RequestType* request, const size_t batchSizeIndex);

Conversation

atobiszei commented Apr 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Diff Breakdown

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

atobiszei Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

atobiszei commented Apr 17, 2026 •

edited

Loading

atobiszei Apr 28, 2026 •

edited

Loading