Merge lp:~verifypn-cpn/verifypn/SuccGen-simp into lp:verifypn

[Andreas Klostergaard]

[Jiri Srba (srba)]

This gets quite less readable. Does it improve the performance (and how much)?

[Peter Gjøl Jensen (peter-gjoel)]

I would change the comparison to:

_parent->marking()[inv.place] >= inv.tokens == inv.inhibitor

and add the comment
// Cannot fire if less tokens than weight (and not inhibitor) or more or equal tokens (and inhibitor).

[lp:~verifypn-cpn/verifypn/SuccGen-simp]

211. By Andreas Klostergaard on 2018-06-12

Fix from comments

[Andreas Klostergaard (aklost11)]

> I would change the comparison to:
>
> _parent->marking()[inv.place] >= inv.tokens == inv.inhibitor
>
> and add the comment
> // Cannot fire if less tokens than weight (and not inhibitor) or more or equal
> tokens (and inhibitor).

This should be fixed now. I changed the comment a little, as I found this to make more sense.

[Andreas Klostergaard (aklost11)]

> This gets quite less readable. Does it improve the performance (and how much)?

It gave about 0.1% performance boost in my tests, but I think it may give more with the changes Peter suggested.

[lp:~verifypn-cpn/verifypn/SuccGen-simp]

212. By Andreas Klostergaard on 2018-06-14

Added cmake support for MacOS

[Peter Gjøl Jensen (peter-gjoel)]

Looks good to me now.

review: Approve

[Jiri Srba (srba)]

I run correctness checks and this is all fine. There is no noticeable improvement in performance
though. I wonder whether we should merge it to trunk or not. What are the benefits compared to decreased readability?

[Peter Gjøl Jensen (peter-gjoel)]

> I run correctness checks and this is all fine. There is no noticeable
> improvement in performance
> though. I wonder whether we should merge it to trunk or not. What are the
> benefits compared to decreased readability?

I am abandoning this branch, the code is less readable and any compiler should optimize this away easily.
I am proposing a different merge with the CMAKE changes.

Unmerged revisions

212. By Andreas Klostergaard on 2018-06-14

Added cmake support for MacOS

211. By Andreas Klostergaard on 2018-06-12

Fix from comments

210. By Andreas Klostergaard on 2018-06-12

Simplifying expression in SuccessorGenerator

=== modified file 'CMakeLists.txt'
--- CMakeLists.txt	2018-04-19 22:14:38 +0000
+++ CMakeLists.txt	2018-06-14 22:37:21 +0000
@@ -1,15 +1,19 @@
 cmake_minimum_required(VERSION 2.8.4)
 project(verifypn)
 
-if (UNIX)
+if (UNIX AND NOT APPLE)
     if (CMAKE_SIZEOF_VOID_P EQUAL 8) # is system 64-bit?
         set(ARCH_TYPE "linux64")
+        set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fPIC -flto -march=x86-64 -std=c++14  -m64 -I.")
+        set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -flto=4 -march=x86-64 -std=c++14 -m64 -static -static-libgcc -static-libstdc++")
     else()
         set(ARCH_TYPE "linux32")
     endif ()
 elseif(APPLE)
     if (CMAKE_SIZEOF_VOID_P EQUAL 8) # is system 64-bit?
         set(ARCH_TYPE "osx64")
+        set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -mmacosx-version-min=10.7 -std=c++14  -m64 -I. -stdlib=libc++")
+        set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -dynamic -mmacosx-version-min=10.7 -std=c++14 -m64 -stdlib=libc++ -lc++")
     else()
         set(ARCH_TYPE "osx32")
     endif ()
@@ -21,11 +25,9 @@
     endif()
 endif ()
 
-set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fPIC -flto -march=x86-64 -std=c++14  -m64 -I.")
 set(CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE} -Wall -pedantic-errors -O2 -DNDEBUG")
 set(CMAKE_CXX_FLAGS_DEBUG  "${CMAKE_CXX_FLAGS_DEBUG} -g")
 
-set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -flto=4 -march=x86-64 -std=c++14 -m64 -static -static-libgcc -static-libstdc++")
 set(CMAKE_EXE_LINKER_FLAGS_RELEASE "${CMAKE_EXE_LINKER_FLAGS_RELEASE} -O2 -DNDEBUG")
 set(CMAKE_EXE_LINKER_FLAGS_DEBUG "${CMAKE_EXE_LINKER_FLAGS_DEBUG} -g")
 
@@ -38,7 +40,7 @@
         "**/*.h"
         "**/**/*.cpp"
         "**/**/*.h"
-)
+        )
 
 add_executable(verifypn ${verifypn_SRC})
 target_link_libraries(verifypn ${CMAKE_SOURCE_DIR}/lpsolve/liblpsolve55-${ARCH_TYPE}.a)

=== modified file 'PetriEngine/SuccessorGenerator.cpp'
--- PetriEngine/SuccessorGenerator.cpp	2018-05-12 15:14:12 +0000
+++ PetriEngine/SuccessorGenerator.cpp	2018-06-14 22:37:21 +0000
@@ -57,14 +57,9 @@
 
         for (; finv < linv; ++finv) {
             const Invariant& inv = _net._invariants[finv];
-            if ((*_parent).marking()[inv.place] < inv.tokens) {
-                if (!inv.inhibitor) {
-                    return false;
-                }
-            } else {
-                if (inv.inhibitor) {
-                    return false;
-                }
+            // Cannot fire if less tokens than weight (when not inhibitor) or more or equal tokens (when inhibitor).
+            if ((*_parent).marking()[inv.place] >= inv.tokens == inv.inhibitor) {
+                return false;
             }
         }
         return true;