Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Read postgresql data on C++ #2607

Merged
merged 71 commits into from
Feb 1, 2024

Conversation

cvvergara
Copy link
Member

@cvvergara cvvergara commented Jan 24, 2024

Moving the input queries from the C code into the C++ code.

Before this change

On the C file:

  • Includes what we call a driver header is linked as C
    • The driver headers are very similar to each other
  • the static process within each C file example are very similar
    • opens the connection with PostgreSQL
    • reads the data, (edges sql, restrictions_sql, etc and any array that is on the query)
      • all of these reading is done on the C code which interacts with PostgreSQL, (which is written in C.)
      • Suppose, for simplifying this description, that the data takes x MB in memory
    • calls the pgr_do_function defined on the driver
    • gets the results
    • closes the connection with PostgreSQL

On the driver C++ file:

  • code is written in C++
  • Drivers are very similar within each other.
    • Convert C arrays to C++ containers
      • Now memory size is 2x + container overhead
  • Builds the boost graph
    • Now memory size is 3x + container overhead
  • None of this code interacts with PostgreSQL

The ideal situation:

  • Build the boost graph as the data is read.
  • Have C++ templates on the drivers
    • Being so similar that can be done

General steps to reach the ideal situation

  1. Be able to read the PostgreSQL data on the C++ code
  2. Create the templates
  3. Build the boost graphs based on the templates needs

This PR is step 1

In rough terms, moving the reading of the data to C++.

The sketch of the C & C++ driver files for the first step:

On the C file:

  • Includes what we call a driver header linked as C
    • The driver headers are very similar to each other
  • the static process within each C file will still be very similar
    • opens the connection with PostgreSQL
    • calls the pgr_do_function defined on the driver
    • gets the results
    • closes the connection with PostgreSQL

On the driver C++ file:

  • code is written in C++
  • Drivers will still be very similar within each other.
    • C++ code will interact with PostgreSQL
    • Read the data directly into C++ containers
      • Two kinds of data
        • Data that comes from the inner SQL queries
        • Data that comes from arrays
      • All of these reading will be do with C++ code
      • Now the reading into C++ container takes x MB in memory
  • Builds the boost graph
    • Now memory size is 2x + container overhead

tasks

  • Copy the files pgdata_getters and pgdata_fetchers to trsp_pgget & trsp_pgfetch
    • Verify that the only function that is including the trsp_pgget is the deprecated pgr_trsp function
    • Remove the unused code on trsp_pgget & trsp_pgfetch
  • Create workaround to postgres defines of functions that exist on the standard
  • Adjust pgdata_getters and pgdata_fetchers to be used directly from C++
    • parameter is a string containing the SQL query
    • return a structure instead of a pointer
    • return a C++ container instead of a pointer
  • Create new template get_data that works with the changes on pgdata_getters and pgdata_fetchers
  • Do general adjustments to several files
  • Per sub directory basis
    • Delete from C file code that reads the data
    • Add into C++ driver file(s) the code that reads the data
    • If necessary adjust the code that use a C array to use the C++ container
  • standardize the driver names to start with pgr_do_
  • Update release_notes & NEWS

@cvvergara cvvergara added this to the Release 3.7.0 milestone Jan 24, 2024
pgdata_getters will be called from C++ code only
- parameter is the sql query
- returns a C++ container
- This include will be removed when the reading takes place on C++
- The only expected place to remain is on old function of trsp
- C files: marking unused code
  - cleanup: removing comments
- _driver.h:
  - parameter for queries is the sql char*
  - Clean up: removing parameter names & comments
- _driver.cpp:
  - c++ code read the data from the SQL queries
  - Adding a catch for input errors
@cvvergara cvvergara force-pushed the read-data-on-cpp-try3 branch from 64bba51 to 3be0767 Compare January 25, 2024 16:16
@cvvergara cvvergara force-pushed the read-data-on-cpp-try3 branch from 47eef4e to d6585aa Compare January 31, 2024 14:13
@cvvergara cvvergara marked this pull request as ready for review January 31, 2024 20:22
@cvvergara cvvergara requested review from sanak and robe2 January 31, 2024 20:23
@cvvergara
Copy link
Member Author

Something happened on the cpplint that is not working anymore on the CI.
But locally it is ok

@cvvergara
Copy link
Member Author

Opened an issue here

@sanak
Copy link
Member

sanak commented Feb 1, 2024

@cvvergara I encountered the following build error on my M1 MacBook Pro environment.

:
[ 60%] Built target bellman_ford
[ 61%] Building CXX object src/cpp_common/CMakeFiles/cpp_common.dir/Dmatrix.cpp.o
[ 61%] Building CXX object src/cpp_common/CMakeFiles/cpp_common.dir/compPaths.cpp.o
[ 62%] Building CXX object src/cpp_common/CMakeFiles/cpp_common.dir/rule.cpp.o
[ 62%] Building CXX object src/cpp_common/CMakeFiles/cpp_common.dir/bpoint.cpp.o
[ 63%] Building CXX object src/cpp_common/CMakeFiles/cpp_common.dir/pgr_messages.cpp.o
[ 63%] Building CXX object src/cpp_common/CMakeFiles/cpp_common.dir/combinations.cpp.o
In file included from /Users/sanak/Projects/pgRouting/git/pgrouting/src/cpp_common/combinations.cpp:27:
In file included from /Users/sanak/Projects/pgRouting/git/pgrouting/include/cpp_common/combinations.hpp:46:
In file included from /Users/sanak/Projects/pgRouting/git/pgrouting/include/cpp_common/basePath_SSEC.hpp:44:
In file included from /opt/homebrew/include/boost/graph/adjacency_list.hpp:20:
In file included from /opt/homebrew/include/boost/unordered_set.hpp:17:
In file included from /opt/homebrew/include/boost/unordered/unordered_set.hpp:17:
In file included from /opt/homebrew/include/boost/unordered/detail/serialize_fca_container.hpp:12:
In file included from /opt/homebrew/include/boost/unordered/detail/serialize_container.hpp:13:
In file included from /opt/homebrew/include/boost/throw_exception.hpp:21:
In file included from /opt/homebrew/include/boost/exception/exception.hpp:9:
/opt/homebrew/include/boost/assert/source_location.hpp:95:9: error: no member named 'snprintf' in namespace 'std'; did you mean simply 'snprintf'?
        BOOST_ASSERT_SNPRINTF( buffer, ":%lu", ln );
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/opt/homebrew/include/boost/assert/source_location.hpp:79:53: note: expanded from macro 'BOOST_ASSERT_SNPRINTF'
# define BOOST_ASSERT_SNPRINTF(buffer, format, arg) std::snprintf(buffer, sizeof(buffer)/sizeof(buffer[0]), format, arg)
                                                    ^~~~~
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.2.sdk/usr/include/stdio.h:337:6: note: 'snprintf' declared here
int      snprintf(char * __restrict __str, size_t __size, const char * __restrict __format, ...) __printflike(3, 4);
         ^
In file included from /Users/sanak/Projects/pgRouting/git/pgrouting/src/cpp_common/combinations.cpp:27:
In file included from /Users/sanak/Projects/pgRouting/git/pgrouting/include/cpp_common/combinations.hpp:46:
In file included from /Users/sanak/Projects/pgRouting/git/pgrouting/include/cpp_common/basePath_SSEC.hpp:44:
In file included from /opt/homebrew/include/boost/graph/adjacency_list.hpp:20:
In file included from /opt/homebrew/include/boost/unordered_set.hpp:17:
In file included from /opt/homebrew/include/boost/unordered/unordered_set.hpp:17:
In file included from /opt/homebrew/include/boost/unordered/detail/serialize_fca_container.hpp:12:
In file included from /opt/homebrew/include/boost/unordered/detail/serialize_container.hpp:13:
In file included from /opt/homebrew/include/boost/throw_exception.hpp:21:
In file included from /opt/homebrew/include/boost/exception/exception.hpp:9:
/opt/homebrew/include/boost/assert/source_location.hpp:102:13: error: no member named 'snprintf' in namespace 'std'; did you mean simply 'snprintf'?
            BOOST_ASSERT_SNPRINTF( buffer, ":%lu", co );
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/opt/homebrew/include/boost/assert/source_location.hpp:79:53: note: expanded from macro 'BOOST_ASSERT_SNPRINTF'
# define BOOST_ASSERT_SNPRINTF(buffer, format, arg) std::snprintf(buffer, sizeof(buffer)/sizeof(buffer[0]), format, arg)
                                                    ^~~~~
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.2.sdk/usr/include/stdio.h:337:6: note: 'snprintf' declared here
int      snprintf(char * __restrict __str, size_t __size, const char * __restrict __format, ...) __printflike(3, 4);
         ^
2 errors generated.
make[2]: *** [src/cpp_common/CMakeFiles/cpp_common.dir/combinations.cpp.o] Error 1
make[1]: *** [src/cpp_common/CMakeFiles/cpp_common.dir/all] Error 2
make: *** [all] Error 2

And changing the following was necessary to pass the build.

--- a/src/cpp_common/combinations.cpp
+++ b/src/cpp_common/combinations.cpp
@@ -24,12 +24,11 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
 
  ********************************************************************PGR-GNU*/
 
-#include "cpp_common/combinations.hpp"
-
 #include <map>
 #include <set>
 #include <deque>
 #include <vector>
+#include "cpp_common/combinations.hpp"
 #include "cpp_common/pgdata_getters.hpp"
 #include "cpp_common/basePath_SSEC.hpp"

After the build, it seems to be fine, but I will check more at tomorrow.

@cvvergara cvvergara merged commit e15a0c5 into pgRouting:develop Feb 1, 2024
22 checks passed
@cvvergara cvvergara deleted the read-data-on-cpp-try3 branch February 1, 2024 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants