From 27ff416327c905a17fcdda3baf4f63a06c198089 Mon Sep 17 00:00:00 2001
From: Nora Abi Akar <nora.abiakar@gmail.com>
Date: Fri, 27 Mar 2020 11:50:15 +0100
Subject: [PATCH] optimize model-init runtime (#985)

`model-init` run-times have been observed to be too high when running systems with many synapses.

The main culprit is the `sort_by` function (fvm_layout:723). When testing on a model with 1024 cells and 10000 synapses, 65% of model-init is spent in this function.

Comparing two `std::map<std::string, double>` is really expensive. We can replace the string map `synapse_instance.param_value` with a sorted `std::vector<std::pair<unsigned, double>>`, and have an additional `std::map<std::string, unsigned> param_map`  to keep track of all the param_name -> unsigned mappings.

`model-init` run-time is 2.5x faster (tested on my 4 core  i7-7600U laptop):
```
10000 synapses/cell
--------------------------------------------------------------------
|  num cells       | model-init before  (s) | model-init after (s) |
--------------------------------------------------------------------
|       1024       |          12            |       5              |
|       2048       |          25.5          |       10.8           |
|       4096       |          55.5          |       23.2           |
|       8192       |          121.8         |       49             |
--------------------------------------------------------------------
```
---
 arbor/fvm_layout.cpp | 28 ++++++++++++++++++++--------
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/arbor/fvm_layout.cpp b/arbor/fvm_layout.cpp
index ffc86d5e..19a68d2e 100644
--- a/arbor/fvm_layout.cpp
+++ b/arbor/fvm_layout.cpp
@@ -684,29 +684,38 @@ fvm_mechanism_data fvm_build_mechanism_data(const cable_cell_global_properties&
 
     struct synapse_instance {
         size_type cv;
-        std::map<std::string, double> param_value; // uses ordering of std::map
+        std::vector<std::pair<unsigned, double>> param_value; // sorted vector of <param_id,value> pairs
         size_type target_index;
     };
 
     for (const auto& entry: cell.synapses()) {
         const std::string& name = entry.first;
         mechanism_info info = catalogue[name];
-        std::map<std::string, double> default_param_value;
         std::vector<synapse_instance> sl;
 
+        // Map from a param string identifier to a unique unsigned int identifier
+        std::map<std::string, unsigned> param_map;
+
+        // Map from a param unsigned int identifier to a default value
+        std::map<unsigned, double> default_param_value;
+
+        unsigned id=0;
         for (const auto& kv: info.parameters) {
-            default_param_value[kv.first] = kv.second.default_value;
+            param_map[kv.first] = id;
+            default_param_value[id++] = kv.second.default_value;
         }
 
         for (const placed<mechanism_desc>& pm: entry.second) {
             verify_mechanism(info, pm.item);
 
             synapse_instance in;
-            in.param_value = default_param_value;
 
+            auto param_value_map = default_param_value;
             for (const auto& kv: pm.item.values()) {
-                in.param_value.at(kv.first) = kv.second;
+                param_value_map.at(param_map.at(kv.first)) = kv.second;
             }
+            std::copy(param_value_map.begin(), param_value_map.end(), std::back_inserter(in.param_value));
+            std::sort(in.param_value.begin(), in.param_value.end());
 
             in.target_index = pm.lid;
             in.cv = D.geometry.location_cv(cell_idx, pm.loc);
@@ -728,7 +737,7 @@ fvm_mechanism_data fvm_build_mechanism_data(const cable_cell_global_properties&
 
         fvm_mechanism_config config;
         config.kind = mechanismKind::point;
-        for (auto& pentry: default_param_value) {
+        for (auto& pentry: param_map) {
             config.param_values.emplace_back(pentry.first, std::vector<value_type>{});
         }
 
@@ -746,9 +755,12 @@ fvm_mechanism_data fvm_build_mechanism_data(const cable_cell_global_properties&
                 }
 
                 unsigned j = 0;
-                for (auto& pentry: in.param_value) {
+                for (auto& pentry: param_map) {
                     arb_assert(config.param_values[j].first==pentry.first);
-                    config.param_values[j++].second.push_back(pentry.second);
+                    auto it = std::lower_bound(in.param_value.begin(), in.param_value.end(), pentry.second, [](auto el, unsigned val) {return el.first < val;});
+
+                    arb_assert(it!= in.param_value.end());
+                    config.param_values[j++].second.push_back((*it).second);
                 }
             }
             config.target.push_back(in.target_index);
-- 
GitLab