Making compilation faster #1893

cvvergara · 2021-06-21T12:51:24Z

State in the opened issue that you will be working on it
Working PR will be merged, non working PR will have a week to be fixed or they will be closed

Making compilation faster

The compilation is slowed down when having an #include "file.h" directive on an ".h" or ".hpp" when the file.h defines a class or structure, if there is no data created with the class or structure defined in "file.h"

In pgRouting C & C++ code use C structures in order to compile due to PostgreSQL been linked with pgRouting's code.

Basically the changes that need to be done are:

On a header file (.h/.hpp):
Change

#include "c_types/matrix_cell_t.h"

Note If the header file has templates leave the line
to:

typedef struct Matrix_cell_t Matrix_cell_t;

On a code file (.c/.cpp) where needed
Add the line:

#include "c_types/matrix_cell_t.h"

List of related files/issues:

General working steps:

Choose the issue (from the list above) that you are planning to solve
State in the issue that is your plan to solve the issue
Work on the issue
After the first commit where you change the file that has the struct
- make a pull request to develop branch as a Draft
- mentioning the issue you are going to solve
After making all adjustments to the code
- commit them and push
- verify the actions work
- Remove the Draft state

Detailed process

This guideline is written using Unix commands.

Create a fork

To develop is better to do it in your own fork, so first you need to
Create a fork of the repository:
https://github.com/pgRouting/pgrouting/tree/develop/
We call fork to the "copy" of the repository
For example this is Vicky's fork:
https://github.com/cvvergara/pgrouting

Create a clone of your fork

A Clone resides on your computer, for example this is done by Vicky:

git clone https://github.com/cvvergara/pgrouting

The command will create a directory named pgrouting
Go to the root of the repository:

cd pgrouting

Make sure the work is done on the current contents

First add as a remote the main repository

git remote add upstream https://github.com/pgRouting/pgrouting
git fetch upstream

choose the issue

Branch out from develop and name the working branch with the issue you are fixing

git checkout upstream/develop
git status

the last message will have the:

You are in 'detached HEAD' state ....

The issues are related to the files in this directory

https://github.com/pgRouting/pgrouting/blob/develop/include/c_types/

From the file:
https://github.com/pgRouting/pgrouting/blob/master/include/c_types/pgr_edge_t.h#L43
pgr_edge_t is the one to fix, lets see how much work it implies:

git grep --name-only pgr_edge_t | wc -l
144

which means that that fix will need to affect 144 files (that is why its not a good first issue)

From:
https://github.com/pgRouting/pgrouting/blob/master/include/c_types/matrix_cell_t.h#L41

git grep --name-only Matrix_cell_t | wc -l
18

This is the one that its going to be used as for the guide which is issue #1903

Create the working branch

Create the working branch, and push. making sure that the actions are executed:

git switch -c issue1903

Go to your fork and click the "Actions" button

edit the file

Using the editor of your preference, edit the file:
include/c_types/matrix_cell_t.h

Change from:

typedef struct matrix_cell {
    int64_t from_vid;
    int64_t to_vid;
    double cost;
} Matrix_cell_t;

remove typedef
the name on the last line is used on after the struct word
the final result:

struct Matrix_cell_t {
    int64_t from_vid;
    int64_t to_vid;
    double cost;
};

In some cases originally the code looks like this:

typedef struct {
    int64_t from_vid;
    int64_t to_vid;
    double cost;
} Matrix_cell_t;

The final result is the same as above

Add the change and commit

git add include/c_types/matrix_cell_t.h
git commit -m '[c_gypes] fixing matrix_cell_t.h'

you can optionally push, but the actions will fail at this point

The compiler is your best friend

prepare for compilation as follows:

mkdir build
cd build
cmake ..

Now, let the compiler tell you the file that needs change:

make

From the compilation:

[  4%] Building C object src/common/CMakeFiles/common.dir/matrixRows_input.c.o
In file included from /home/vicky/pgrouting/pgrouting/cvvergara/src/common/matrixRows_input.c:25:
/home/vicky/pgrouting/pgrouting/cvvergara/include/c_common/matrixRows_input.h:40:9: error: expected declaration specifiers or ‘...’ before ‘Matrix_cell_t’
         Matrix_cell_t **distaces,
         ^~~~~~~~~~~~~
/home/vicky/pgrouting/pgrouting/cvvergara/src/common/matrixRows_input.c:45:9: error: expected declaration specifiers or ‘...’ before ‘Matrix_cell_t’
         Matrix_cell_t *distance) {
         ^~~~~~~~~~~~~
/home/vicky/pgrouting/pgrouting/cvvergara/src/common/matrixRows_input.c:58:9: error: expected declaration specifiers or ‘...’ before ‘Matrix_cell_t’
         Matrix_cell_t **rows,

there are 2 files that need adjustment:

include/c_common/matrixRows_input.h
src/common/matrixRows_input.c

fixing a header

Always start with the header file in this case include/c_common/matrixRows_input.h

This needs to be modified, basically instead of the include directive have a typedef
https://github.com/pgRouting/pgrouting/blob/develop/include/c_common/matrixRows_input.h#L31
change:

#include "c_types/matrix_cell_t.h"

to:

typedef struct Matrix_cell_t Matrix_cell_t;

fixing the code

In the C/C++ file is where the #include "c_types/matrix_cell_t.h" line should go
https://github.com/pgRouting/pgrouting/blob/develop/src/common/matrixRows_input.c#L33
preferably before common includes
For example after adding to the file src/common/matrixRows_input.c the line:

...
#include "c_types/column_info_t.h"
#include "c_types/matrix_cell_t.h"
...

compile again

make

Some times there is a lot of things going on:

[ 17%] Building C object src/allpairs/CMakeFiles/allpairs.dir/floydWarshall.c.o
In file included from /home/vicky/pgrouting/pgrouting/cvvergara/src/allpairs/floydWarshall.c:40:
/home/vicky/pgrouting/pgrouting/cvvergara/include/drivers/allpairs/floydWarshall_driver.h:55:5: error: unknown type name ‘Matrix_cell_t’
     Matrix_cell_t **ret_matrix,
     ^~~~~~~~~~~~~
/home/vicky/pgrouting/pgrouting/cvvergara/src/allpairs/floydWarshall.c:50:9: error: unknown type name ‘Matrix_cell_t’
         Matrix_cell_t **result_tuples,
         ^~~~~~~~~~~~~
/home/vicky/pgrouting/pgrouting/cvvergara/src/allpairs/floydWarshall.c: In function ‘_pgr_floydwarshall’:
/home/vicky/pgrouting/pgrouting/cvvergara/src/allpairs/floydWarshall.c:108:5: error: unknown type name ‘Matrix_cell_t’; use ‘struct’ keyword to refer to the type
     Matrix_cell_t  *result_tuples = NULL;
     ^~~~~~~~~~~~~
     struct 
/home/vicky/pgrouting/pgrouting/cvvergara/src/allpairs/floydWarshall.c:132:9: warning: implicit declaration of function ‘process’ [-Wimplicit-function-declaration]
         process(
         ^~~~~~~
/home/vicky/pgrouting/pgrouting/cvvergara/src/allpairs/floydWarshall.c:160:22: error: ‘Matrix_cell_t’ undeclared (first use in this function)
     result_tuples = (Matrix_cell_t*) funcctx->user_fctx;
                      ^~~~~~~~~~~~~
/home/vicky/pgrouting/pgrouting/cvvergara/src/allpairs/floydWarshall.c:160:22: note: each undeclared identifier is reported only once for each function it appears in
/home/vicky/pgrouting/pgrouting/cvvergara/src/allpairs/floydWarshall.c:160:36: error: expected expression before ‘)’ token
     result_tuples = (Matrix_cell_t*) funcctx->user_fctx;
                                    ^
In file included from /home/vicky/pgrouting/pgrouting/cvvergara/include/c_common/postgres_connection.h:29,
                 from /home/vicky/pgrouting/pgrouting/cvvergara/src/allpairs/floydWarshall.c:32:
/home/vicky/pgrouting/pgrouting/cvvergara/src/allpairs/floydWarshall.c:173:68: error: request for member ‘from_vid’ in something not a structure or union
         values[0] = Int64GetDatum(result_tuples[funcctx->call_cntr].from_vid);
                                                                    ^
/home/vicky/pgrouting/pgrouting/cvvergara/src/allpairs/floydWarshall.c:175:68: error: request for member ‘to_vid’ in something not a structure or union
         values[1] = Int64GetDatum(result_tuples[funcctx->call_cntr].to_vid);
                                                                    ^
/home/vicky/pgrouting/pgrouting/cvvergara/src/allpairs/floydWarshall.c:177:69: error: request for member ‘cost’ in something not a structure or union
         values[2] = Float8GetDatum(result_tuples[funcctx->call_cntr].cost);

But again lets detect the files that need to be modified:

include/drivers/allpairs/floydWarshall_driver.h
- header file
- change #include "c_types/matrix_cell_t.h" to typedef struct Matrix_cell_t Matrix_cell_t;
src/allpairs/floydWarshall.c
- code file
- add line #include "c_types/matrix_cell_t.h"
  - if there is a: #include "c_common/postgres_connection.h" line put the new line after it

compile again

make

the first lines of the error is:

In file included from /home/vicky/pgrouting/pgrouting/cvvergara/src/allpairs/floydWarshall_driver.cpp:36:
/home/vicky/pgrouting/pgrouting/cvvergara/include/allpairs/pgr_allpairs.hpp: In member function ‘void Pgr_allpairs<G>::make_result(const G&, const std::vector<std::vector<double> >&, size_t&, Matrix_cell_t**) const’:
/home/vicky/pgrouting/pgrouting/cvvergara/include/allpairs/pgr_allpairs.hpp:207:43: warning: invalid use of incomplete type ‘Matrix_cell_t’ {aka ‘struct Matrix_cell_t’}
                      (*postgres_rows)[seq].from_vid = graph[v_i].id;

and now detecting include/allpairs/pgr_allpairs.hpp as the next file to edit, which is a header

and compile again

include/drivers/allpairs/johnson_driver.h
- header file
- change #include "c_types/matrix_cell_t.h" to typedef struct Matrix_cell_t Matrix_cell_t;
src/allpairs/johnson.c
- code file
- add line #include "c_types/matrix_cell_t.h"
  - if there is a: #include "c_common/postgres_connection.h" line put the new line after it

Huston we have a problem:

From the next compilation:

[ 28%] Building CXX object src/tsp/CMakeFiles/tsp.dir/TSP_driver.cpp.o
In file included from /home/vicky/pgrouting/pgrouting/cvvergara/src/tsp/TSP_driver.cpp:38:
/home/vicky/pgrouting/pgrouting/cvvergara/include/cpp_common/Dmatrix.h:123:36: error: ‘matrix_cell’ was not declared in this scope
     void set_ids(const std::vector<matrix_cell> &data_costs);
                                    ^~~~~~~~~~~
/home/vicky/pgrouting/pgrouting/cvvergara/include/cpp_common/Dmatrix.h:123:36: note: suggested alternative: ‘Matrix_cell_t’
     void set_ids(const std::vector<matrix_cell> &data_costs);
                                    ^~~~~~~~~~~
                                    Matrix_cell_t
/home/vicky/pgrouting/pgrouting/cvvergara/include/cpp_common/Dmatrix.h:123:47: error: template argument 1 is invalid
     void set_ids(const std::vector<matrix_cell> &data_costs);

include/cpp_common/Dmatrix.h
- change #include "c_types/matrix_cell_t.h" to typedef struct Matrix_cell_t Matrix_cell_t;
  But also change the matrix_cell to Matrix_cell_t to be consistent

its time for a break or it compiles completly:

commit and push

# to verify that no other file was modified accidentally
git status  
git commit -a -m '[C/C++] some adjustments because of change on matrix_cell_t.h'
git push

Cleaning up the PR

doing a git status yields something like:

commit c373f82b6af3668d4c3713f9a7227a31c3453b59 (HEAD -> issue1903, issue1093)
Author: cvvergara <vicky@georepublic.de>
Date:   Fri Jun 18 15:10:45 2021 -0500

    [C/C++] some adjustments because of change on matrix_cell_t.h

commit 76ca87aeac7004866259934fc22a5c97d68d3b02
Author: cvvergara <vicky@georepublic.de>
Date:   Fri Jun 18 15:05:18 2021 -0500

    [C/C++] some adjustments because of change on matrix_cell_t.h

commit 03a92a4d2c222af1c428fe28368460e7b44e9211
Author: cvvergara <vicky@georepublic.de>
Date:   Fri Jun 18 13:15:33 2021 -0500

    [c_types] fixing matrix_cell_t.h

commit a841d149e0024eb5d5cf7cab1d661c3d42b829a5 (upstream/develop, develop)

The Pull request should have 2 commits at most, during this work 3 commits were done maybe because of a break, maybe to save work done.

git fetch upstream
git rebase -i upstream/develop

That will open an editor with content like:

pick 03a92a4d2c [c_types] fixing matrix_cell_t.h
pick 76ca87aeac [C/C++] some adjustments because of change on matrix_cell_t.h
pick c373f82b6a [C/C++] some adjustments because of change on matrix_cell_t.h

note: read the help that is on the file

in this example:

remove the word "some"
keep the adjustments in one commit

is done as follows:

pick 03a92a4d2c [c_types] fixing matrix_cell_t.h
reword 76ca87aeac [C/C++] some adjustments because of change on matrix_cell_t.h
fixup c373f82b6a [C/C++] some adjustments because of change on matrix_cell_t.h

exit the file saving the changes.
It will open another file to edit where the word "some" can be removed. Remove it, and exit the file saving the changes

force the push as follows:

git push -f

Please inspect the PR #1931 for this demonstration

The text was updated successfully, but these errors were encountered:

krashish8 · 2021-08-27T15:56:07Z

I think the two leftover tasks of this issue are also done - trsp (#2059), pgr_edge_xy_t (#2062), and this issue can be closed.
@cvvergara You may close it after confirming.

cvvergara · 2021-08-27T16:18:43Z

@krashish8 Thanks for reviewing

cvvergara added good first issue OSD Open Source Day labels Jun 21, 2021

This was referenced Jun 21, 2021

Making compilation faster include/c_types/column_info_t.h #1894

Closed

Making compilation faster include/c_types/contracted_rt.h #1895

Closed

cvvergara added this to the Release 3.3.0 milestone Jun 21, 2021

This was referenced Jul 21, 2021

fix for Issue1926 Restriction_t #2029

Closed

[c_type] fixing pgr_mst_rt #2034

Closed

cvvergara closed this as completed Aug 27, 2021

cvvergara mentioned this issue Aug 28, 2021

C types reorganization and cleanup #2103

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Making compilation faster #1893

Making compilation faster #1893

cvvergara commented Jun 21, 2021 •

edited

krashish8 commented Aug 27, 2021

cvvergara commented Aug 27, 2021

Making compilation faster #1893

Making compilation faster #1893

Comments

cvvergara commented Jun 21, 2021 • edited

Making compilation faster

Basically the changes that need to be done are:

General working steps:

Detailed process

Create a fork

Create a clone of your fork

Make sure the work is done on the current contents

choose the issue

The issues are related to the files in this directory

Create the working branch

edit the file

The compiler is your best friend

fixing a header

fixing the code

compile again

Huston we have a problem:

its time for a break or it compiles completly:

Cleaning up the PR

krashish8 commented Aug 27, 2021

cvvergara commented Aug 27, 2021

cvvergara commented Jun 21, 2021 •

edited