Little refactoring of OrdVector

include/mata/nfa/delta.hh

@@ -81,11 +81,11 @@ public:
     // but useful for adding states in a random order to sort later (supposedly more efficient than inserting in a random order)
     void inline push_back(const State s) { targets.push_back(s); }
 
-    void remove(State s) { targets.remove(s); }
+    void erase(State s) { targets.erase(s); }
 
     std::vector<State>::const_iterator find(State s) const { return targets.find(s); }
     std::vector<State>::iterator find(State s) { return targets.find(s); }
-}; // class mata::nfa::Move.
+}; // class mata::nfa::SymbolPost.
 
 /**
  * @brief A data structure representing possible transitions over different symbols from a source state.
@@ -107,16 +107,17 @@ public:
     StatePost& operator=(StatePost&&) = default;
     using super::insert;
     using super::reserve;
-    using super::remove;
     using super::empty, super::size;
     using super::ToVector;
-    using super::erase;
     // dangerous, breaks the sortedness invariant
     using super::push_back;
     // is adding non-const version as well ok?
     using super::back;
     using super::filter;
 
+    void erase(const SymbolPost& s) {super::erase(s);}
+    void erase(const_iterator first, const_iterator last) {super::erase(first,last);}
+
     using super::find;
     iterator find(const Symbol symbol) { return super::find({ symbol, {} }); }
     const_iterator find(const Symbol symbol) const { return super::find({ symbol, {} }); }
@@ -238,7 +239,7 @@ public:
         }
         StateSet bigger_succ = {};
         for (const auto m: this->get_current()) {
-            bigger_succ = bigger_succ.Union(m->targets);
+            bigger_succ.insert(m->targets);
         }
         return bigger_succ;
     }

include/mata/utils/closed-set.hh

@@ -326,7 +326,7 @@ void ClosedSet<T>::insert(const Node& node) {
     }
 
     for(auto element : to_erase) {
-        antichain_.remove(element);
+        antichain_.erase(element);
     }
     antichain_.insert(node);
 } // insert }}}
@@ -361,9 +361,11 @@ ClosedSet<T> ClosedSet<T>::intersection(const ClosedSet<T>& rhs) const {
     // Iterates through all the tuples from Antichan1 X Antichan2
     // and creates an union of them
     if(type_ == ClosedSetType::upward_closed_set) {
-        for(auto element1 : antichain_) {
-            for(auto element2 : rhs.antichain()) {
-               result.insert(element1.Union(element2));
+        for(const Node& element1 : antichain_) {
+            for(const Node& element2 : rhs.antichain()) {
+                Node tmp = element1;
+                tmp.insert(element2);
+                result.insert(tmp);
             }
         }
     }

include/mata/utils/ord-vector.hh

@@ -138,14 +138,25 @@ public:
         vec_.insert(itr,x);
     }
 
-    // PUSH_BACK WHICH BREAKS SORTEDNESS,
+    // EMPLACE_BACK WHICH BREAKS SORTEDNESS,
     // dangerous,
     // but useful in NFA where temporarily breaking the sortedness invariant allows for a faster algorithm (e.g. revert)
-    virtual inline void push_back(const Key& x) {
-        reserve_on_insert(vec_);
-        vec_.emplace_back(x);
+    template <typename... Args>
+    reference emplace_back(Args&&... args) {
+       return vec_.emplace_back(std::forward<Args>(args)...);
     }
 
+    // PUSH_BACK WHICH BREAKS SORTEDNESS,
+    // dangerous,
+    // but useful in NFA where temporarily breaking the sortedness invariant allows for a faster algorithm (e.g. revert)
+    reference push_back(const Key& t) { return emplace_back(t); }
+
+    // PUSH_BACK WHICH BREAKS SORTEDNESS,
+    // dangerous,
+    // but useful in NFA where temporarily breaking the sortedness invariant allows for a faster algorithm (e.g. revert)
+    // btw, do we need move here?
+    reference push_back(Key&& t) { return emplace_back(std::move(t)); }
+
     virtual inline void reserve(size_t  size) { vec_.reserve(size); }
 
     virtual inline void erase(const_iterator first, const_iterator last) { vec_.erase(first, last); }
@@ -155,52 +166,63 @@ public:
 
         reserve_on_insert(vec_);
 
-        // perform binary search (cannot use std::binary_search because it is
-        // ineffective due to not returning the iterator to the position of the
-        // desirable insertion in case the searched element is not present in the
-        // range)
-        size_t first = 0;
-        size_t last = vec_.size();
-
-        if ((last != 0) && (vec_.back() < x))
-        {	// for the case which would be prevalent
-            // that is, the added thing can is larger than the largest thing and can be just bushed back
-            vec_.push_back(x);
-            return;
-        }
-
-        while (first < last)
-        {	// while the pointers do not overlap
-            size_t middle = first + (last - first) / 2;
-            if (vec_[middle] == x)
-            {	// in case we found x
-                return;
-            }
-            else if (vec_[middle] < x)
-            {	// in case middle is less than x
-                first = middle + 1;
-            }
-            else
-            {	// in case middle is greater than x
-                last = middle;
-            }
-        }
-
-        vec_.resize(vec_.size() + 1);
-        std::copy_backward(vec_.begin() + static_cast<long>(first), vec_.end() - 1, vec_.end());
-
-        // insert the new element
-        vec_[first] = x;
+        // Tomas Fiedor: article about better binary search here https://mhdm.dev/posts/sb_lower_bound/.
+        auto pos  = std::lower_bound(vec_.begin(), vec_.end(), x);
+        if (pos == vec_.end() || *pos != x)
+            vec_.insert(pos,x);
+
+        //TODO: measure, if there is not benefit to this custom version, remove
+        //size_t first = 0;
+        //size_t last = vec_.size();
+
+        //if ((last != 0) && (vec_.back() < x))
+        //{	// for the case which would be prevalent
+        //    // that is, the added thing can is larger than the largest thing and can be just bushed back
+        //    vec_.push_back(x);
+        //    return;
+        //}
+
+        //while (first < last)
+        //{	// while the pointers do not overlap
+        //    size_t middle = first + (last - first) / 2;
+        //    if (vec_[middle] == x)
+        //    {	// in case we found x
+        //        return;
+        //    }
+        //    else if (vec_[middle] < x)
+        //    {	// in case middle is less than x
+        //        first = middle + 1;
+        //    }
+        //    else
+        //    {	// in case middle is greater than x
+        //        last = middle;
+        //    }
+        //}
+
+        //vec_.resize(vec_.size() + 1);
+        //std::copy_backward(vec_.begin() + static_cast<long>(first), vec_.end() - 1, vec_.end());
+
+        //// insert the new element
+        //vec_[first] = x;
 
         assert(vectorIsSorted());
     }
 
     virtual void insert(const OrdVector& vec) {
+        static OrdVector tmp{};
         assert(vectorIsSorted());
         assert(vec.vectorIsSorted());
+        tmp.clear();
+
+        set_union(*this,vec,tmp);
 
-        // TODO: sometimes it is not efficient to copy both vectors to create a new one
-        vec_ = OrdVector::Union(*this, vec).vec_;
+        vec_ = tmp.vec_;
+        /*
+         * Swap, commented below (test do pass) may be better in some cases, such as uniting many vectors into one.
+         * But it defeats the idea of having tmp vector shared between all the unions and allocated only once.
+         * It would be good to try it with removing epsilon transitions or determinization.
+         */
+        //std::swap(tmp.vec_,vec_);
         assert(vectorIsSorted());
     }
 
@@ -228,8 +250,6 @@ public:
 
     OrdVector intersection(const OrdVector& rhs) const { return intersection(*this, rhs); }
 
-    OrdVector Union(const OrdVector& rhs) const { return Union(*this, rhs); }
-
     //TODO: this code of find was duplicated, not nice.
     // Replacing the original code by std function, but keeping the original here commented, it was nice, might be even better.
     virtual const_iterator find(const Key& key) const {
@@ -263,7 +283,7 @@ public:
      *
      * This function expects the vector to be sorted.
      */
-    inline void remove(const Key& k) {
+    inline void erase(const Key& k) {
         assert(vectorIsSorted());
         auto found_value_it = std::lower_bound(vec_.begin(), vec_.end(), k);
         if (found_value_it != vec_.end()) {
@@ -286,8 +306,8 @@ public:
     }
 
     // Indexes with content which is staying are shifted left to take place of indexes with content that is not staying.
-    template<typename F>
-    void filter(F && is_staying) {
+    template<typename Fun>
+    void filter(const Fun && is_staying) {
         utils::filter(vec_, is_staying);
     }
 
@@ -408,38 +428,48 @@ public:
         return result;
     }
 
-    static OrdVector Union(const OrdVector& lhs, const OrdVector& rhs) {
+    static void set_union(const OrdVector& lhs, const OrdVector& rhs, OrdVector& result) {
         assert(lhs.vectorIsSorted());
         assert(rhs.vectorIsSorted());
 
-        OrdVector result;
-
-        auto lhs_it = lhs.begin();
-        auto rhs_it = rhs.vec_.begin();
-
-        while ((lhs_it != lhs.end()) || (rhs_it != rhs.end())) { // Until we get to the end of both vectors.
-            if (lhs_it == lhs.end()) { // If we are finished with the left-hand side vector.
-                result.push_back(*rhs_it);
-                ++rhs_it;
-            } else if (rhs_it == rhs.end()) { // If we are finished with the right-hand side vector.
-                result.push_back(*lhs_it);
-                ++lhs_it;
-            } else {
-                if (*lhs_it < *rhs_it) {
-                    result.push_back(*lhs_it);
-                    ++lhs_it;
-                } else if (*rhs_it < *lhs_it) {
-                    result.push_back(*rhs_it);
-                    ++rhs_it;
-                } else { // In case they are equal.
-                    result.push_back(*rhs_it);
-                    ++rhs_it;
-                    ++lhs_it;
-                }
-            }
-        }
+        if (lhs.empty()) { result = rhs; return; }
+        if (rhs.empty()) { result = lhs; return; }
+
+        result.reserve(lhs.size()+rhs.size());
+        std::set_union(lhs.vec_.begin(),lhs.vec_.end(),rhs.vec_.begin(),rhs.vec_.end(),std::back_inserter(result.vec_));
+
+        //TODO: measure, if there is not benefit to this custom version, remove
+        //auto lhs_it = lhs.begin();
+        //auto rhs_it = rhs.vec_.begin();
+
+        //while ((lhs_it != lhs.end()) || (rhs_it != rhs.end())) { // Until we get to the end of both vectors.
+        //    if (lhs_it == lhs.end()) { // If we are finished with the left-hand side vector.
+        //        result.push_back(*rhs_it);
+        //        ++rhs_it;
+        //    } else if (rhs_it == rhs.end()) { // If we are finished with the right-hand side vector.
+        //        result.push_back(*lhs_it);
+        //        ++lhs_it;
+        //    } else {
+        //        if (*lhs_it < *rhs_it) {
+        //            result.push_back(*lhs_it);
+        //            ++lhs_it;
+        //        } else if (*rhs_it < *lhs_it) {
+        //            result.push_back(*rhs_it);
+        //            ++rhs_it;
+        //        } else { // In case they are equal.
+        //            result.push_back(*rhs_it);
+        //            ++rhs_it;
+        //            ++lhs_it;
+        //        }
+        //    }
+        //}
 
         assert(result.vectorIsSorted());
+    }
+
+    static OrdVector set_union(const OrdVector& lhs, const OrdVector& rhs) {
+        OrdVector result{};
+        set_union(lhs, rhs, result);
         return result;
     }