Merge pull request #28376 from vespa-engine/balder/minor-cleanup

Unify and modernize code and layout

2 files changed, 57 insertions, 28 deletions

diff --git a/eval/src/apps/analyze_onnx_model/analyze_onnx_model.cpp b/eval/src/apps/analyze_onnx_model/analyze_onnx_model.cpp
index a3aa7cbb32f..03db333d582 100644
--- a/eval/src/apps/analyze_onnx_model/analyze_onnx_model.cpp
+++ b/eval/src/apps/analyze_onnx_model/analyze_onnx_model.cpp
@@ -9,6 +9,7 @@
 #include <vespa/vespalib/util/require.h>
 #include <vespa/vespalib/util/guard.h>
 #include <vespa/vespalib/util/stringfmt.h>
+#include <charconv>
 
 using vespalib::make_string_short::fmt;
 
@@ -20,7 +21,12 @@ using vespalib::FilePointer;
 using namespace vespalib::eval;
 using namespace vespalib::eval::test;
 
-struct MyError {
+struct MyError : public std::exception {
+    explicit MyError(vespalib::stringref m) :
+        std::exception(),
+        msg(m)
+    {}
+    const char * what() const noexcept override { return msg.c_str(); }
     vespalib::string msg;
 };
 
@@ -47,17 +53,42 @@ void extract(const vespalib::string &str, const vespalib::string &prefix, vespal
         dst = str.substr(pos);
     }
 }
+struct MemoryUsage {
+    size_t size;
+    size_t rss;
+};
 
-void report_memory_usage(const vespalib::string &desc) {
-    vespalib::string vm_size = "unknown";
-    vespalib::string vm_rss = "unknown";
-    vespalib::string line;
+static const vespalib::string UNKNOWN = "unknown";
+
+size_t convert(const vespalib::string & s) {
+    if (s == UNKNOWN) return 0;
+    size_t v(0);
+    size_t end = s.find("kB");
+    auto [ptr,ec] = std::from_chars(s.data(), s.data()+std::min(s.size(), end), v, 10);
+    if (ec != std::errc()) {
+        throw std::runtime_error(fmt("Bad format : '%s' at '%s'", s.c_str(), ptr));
+    }
+    if (end == vespalib::string::npos) {
+        throw std::runtime_error(fmt("Bad format : %s", s.c_str()));
+    }
+    return v * 1024;
+}
+
+MemoryUsage extract_memory_usage() {
+    vespalib::string vm_size = UNKNOWN;
+    vespalib::string vm_rss = UNKNOWN;
     FilePointer file(fopen("/proc/self/status", "r"));
+    vespalib::string line;
     while (read_line(file, line)) {
         extract(line, "VmSize:", vm_size);
         extract(line, "VmRSS:", vm_rss);
     }
-    fprintf(stderr, "vm_size: %s, vm_rss: %s (%s)\n", vm_size.c_str(), vm_rss.c_str(), desc.c_str());
+    return {convert(vm_size), convert(vm_rss)};
+}
+
+void report_memory_usage(const vespalib::string &desc) {
+    MemoryUsage vm = extract_memory_usage();
+    fprintf(stderr, "vm_size: %zu kB, vm_rss: %zu kB (%s)\n", vm.size/1024, vm.rss/1024, desc.c_str());
 }
 
 struct Options {
@@ -118,7 +149,7 @@ void dump_wire_info(const Onnx::WireInfo &wire) {
 struct MakeInputType {
     Options &opts;
     std::map<vespalib::string,int> symbolic_sizes;
-    MakeInputType(Options &opts_in) : opts(opts_in), symbolic_sizes() {}
+    explicit MakeInputType(Options &opts_in) : opts(opts_in), symbolic_sizes() {}
     ValueType operator()(const Onnx::TensorInfo &info) {
         int d = 0;
         std::vector<ValueType::Dimension> dim_list;
@@ -229,30 +260,32 @@ int probe_types() {
     if (!JsonFormat::decode(std_in, params)) {
         throw MyError{"invalid json"};
     }
+    MemoryUsage vm_before = extract_memory_usage();
     Slime result;
     auto &root = result.setObject();
     auto &types = root.setObject("outputs");
-    Onnx model(params["model"].asString().make_string(), Onnx::Optimize::DISABLE);
+    Onnx model(params["model"].asString().make_string(), Onnx::Optimize::ENABLE);
     Onnx::WirePlanner planner;
-    for (size_t i = 0; i < model.inputs().size(); ++i) {
-        auto spec = params["inputs"][model.inputs()[i].name].asString().make_string();
+    for (const auto & input : model.inputs()) {
+        auto spec = params["inputs"][input.name].asString().make_string();
         auto input_type = ValueType::from_spec(spec);
         if (input_type.is_error()) {
-            if (!params["inputs"][model.inputs()[i].name].valid()) {
-                throw MyError{fmt("missing type for model input '%s'",
-                                  model.inputs()[i].name.c_str())};
+            if (!params["inputs"][input.name].valid()) {
+                throw MyError(fmt("missing type for model input '%s'", input.name.c_str()));
             } else {
-                throw MyError{fmt("invalid type for model input '%s': '%s'",
-                                  model.inputs()[i].name.c_str(), spec.c_str())};
+                throw MyError(fmt("invalid type for model input '%s': '%s'",input.name.c_str(), spec.c_str()));
             }
         }
-        bind_input(planner, model.inputs()[i], input_type);
+        bind_input(planner, input, input_type);
     }
     planner.prepare_output_types(model);
     for (const auto &output: model.outputs()) {
         auto output_type = make_output(planner, output);
         types.setString(output.name, output_type.to_spec());
     }
+    MemoryUsage vm_after = extract_memory_usage();
+    root.setLong("vm_size", vm_after.size - vm_before.size);
+    root.setLong("vm_rss", vm_after.rss - vm_before.rss);
     write_compact(result, std_out);
     return 0;
 }
diff --git a/eval/src/vespa/eval/onnx/onnx_wrapper.cpp b/eval/src/vespa/eval/onnx/onnx_wrapper.cpp
index 8f9450c2660..89d88dcc32c 100644
--- a/eval/src/vespa/eval/onnx/onnx_wrapper.cpp
+++ b/eval/src/vespa/eval/onnx/onnx_wrapper.cpp
@@ -8,10 +8,6 @@
 #include <vespa/vespalib/util/stringfmt.h>
 #include <vespa/vespalib/util/typify.h>
 #include <vespa/vespalib/util/classname.h>
-#include <assert.h>
-#include <cmath>
-#include <stdlib.h>
-#include <stdio.h>
 #include <type_traits>
 
 #include <vespa/log/log.h>
@@ -171,15 +167,15 @@ private:
 public:
     OnnxString(const OnnxString &rhs) = delete;
     OnnxString &operator=(const OnnxString &rhs) = delete;
-    OnnxString(OnnxString &&rhs) = default;
-    OnnxString &operator=(OnnxString &&rhs) = default;
+    OnnxString(OnnxString &&rhs) noexcept = default;
+    OnnxString &operator=(OnnxString &&rhs) noexcept = default;
     const char *get() const { return _str.get(); }
     ~OnnxString() = default;
     static OnnxString get_input_name(const Ort::Session &session, size_t idx) {
-        return OnnxString(session.GetInputNameAllocated(idx, _alloc));
+        return {session.GetInputNameAllocated(idx, _alloc)};
     }
     static OnnxString get_output_name(const Ort::Session &session, size_t idx) {
-        return OnnxString(session.GetOutputNameAllocated(idx, _alloc));
+        return {session.GetOutputNameAllocated(idx, _alloc)};
     }
 };
 Ort::AllocatorWithDefaultOptions OnnxString::_alloc;
@@ -216,7 +212,7 @@ Onnx::TensorType get_type_of(const Ort::Value &value) {
             throw Ort::Exception("[onnx wrapper] actual value has unknown dimension size", ORT_FAIL);
         }
     }
-    return Onnx::TensorType(make_element_type(element_type), shape);
+    return {make_element_type(element_type), shape};
 }
 
 std::vector<int64_t> extract_sizes(const ValueType &type) {
@@ -306,7 +302,7 @@ Onnx::WirePlanner::do_model_probe(const Onnx &model)
             result_values.emplace_back(nullptr);
         }
         Ort::RunOptions run_opts(nullptr);
-        Ort::Session &session = const_cast<Ort::Session&>(model._session);
+        auto &session = const_cast<Ort::Session&>(model._session);
         session.Run(run_opts,
                     model._input_name_refs.data(), param_values.data(), param_values.size(),
                     model._output_name_refs.data(), result_values.data(), result_values.size());
@@ -554,7 +550,7 @@ Onnx::EvalContext::EvalContext(const Onnx &model, const WireInfo &wire_info)
         const auto &vespa = _wire_info.vespa_inputs[i];
         const auto &onnx = _wire_info.onnx_inputs[i];
         if (is_same_type(vespa.cell_type(), onnx.elements)) {
-            _param_values.push_back(Ort::Value(nullptr));
+            _param_values.emplace_back(nullptr);
             _param_binders.push_back(SelectAdaptParam()(vespa.cell_type()));
         } else {
             _param_values.push_back(CreateOnnxTensor()(onnx, _alloc));
@@ -587,7 +583,7 @@ Onnx::EvalContext::bind_param(size_t i, const Value &param)
 void
 Onnx::EvalContext::eval()
 {
-    Ort::Session &session = const_cast<Ort::Session&>(_model._session);
+    auto &session = const_cast<Ort::Session&>(_model._session);
     Ort::RunOptions run_opts(nullptr);
     session.Run(run_opts,
                 _model._input_name_refs.data(), _param_values.data(), _param_values.size(),