pgrouting icon indicating copy to clipboard operation
pgrouting copied to clipboard

Clang tidy perf

Open lbartoletti opened this issue 3 years ago • 5 comments

Hello,

Building pgRouting I see some optimizations spotted by clang.

This fix has been generated by run-clang-tidy -performance=* -fix. It's mostly the use of const ref and std::move perf.

@pgRouting/admins

lbartoletti avatar Aug 02 '22 12:08 lbartoletti

@lbartoletti

Thanks, Just two things:

  1. Your name Can you put here your name: https://github.com/lbartoletti/pgrouting/blob/clang_tidy_perf/doc/src/pgRouting-introduction.rst under "This Release Contributors" and under "Contributors Past & Present"

  2. Issue related PR Can you open an issue about Clang performance so that we can relate this PR to the issue. And here under v3.3.2: https://github.com/lbartoletti/pgrouting/blob/clang_tidy_perf/doc/src/release_notes.rst#pgrouting-332-release-notes Mention the issue

After modifying the release_notes.rst file execute this perl script to update the NEWS automatically

tools/release-scripts/notes2news.pl

cvvergara avatar Aug 05 '22 13:08 cvvergara

Un-draft when your are done. (click on Ready for review)

cvvergara avatar Aug 05 '22 13:08 cvvergara

We will squash, the contribution, to make the porting of the changes to develop branch easier. You might just want to do the squash your self, keeping all the commit messages. Suppose that the main repo you have it under "upstream":

git rebase -i upstream/main
# edit the file: choose "fixup"
git push -f

Then we will only do a rebase instead of a squash

After merging

The commit gets cherry-picked into develop And a PR to develop gets created

Please let me know if you also want to do this step.

cvvergara avatar Aug 05 '22 14:08 cvvergara

We will squash, the contribution, to make the porting of the changes to develop branch easier. You might just want to do the squash your self, keeping all the commit messages. Suppose that the main repo you have it under "upstream":

git rebase -i upstream/main
# edit the file: choose "fixup"
git push -f

Then we will only do a rebase instead of a squash

After merging

The commit gets cherry-picked into develop And a PR to develop gets created

Please let me know if you also want to do this step.

done

lbartoletti avatar Aug 08 '22 08:08 lbartoletti

@lbartoletti

Thanks, Just two things:

1. Your name
   Can you put here your name:
   https://github.com/lbartoletti/pgrouting/blob/clang_tidy_perf/doc/src/pgRouting-introduction.rst
   under "This Release Contributors" and under "Contributors Past & Present"

2. Issue related PR
   Can you open an issue about Clang performance so that we can relate this PR to the issue.
   And here under v3.3.2:
   https://github.com/lbartoletti/pgrouting/blob/clang_tidy_perf/doc/src/release_notes.rst#pgrouting-332-release-notes
   Mention the issue

After modifying the release_notes.rst file execute this perl script to update the NEWS automatically

tools/release-scripts/notes2news.pl

Done in:

  • https://github.com/pgRouting/pgrouting/pull/2356/commits/4f9a07d481d5b06ddbe0f485bfcad9bd6dad9450
  • https://github.com/pgRouting/pgrouting/pull/2356/commits/4706da62578c992eac823b1fdb566f9c2ca2e0bc

BTW the tools/release-scripts/notes2news.pl script is not standard, it should be:

#!/usr/bin/env perl

use warnings;
...
@@ -1,5 +1,6 @@
-#!/usr/bin/perl -w
+#!/usr/bin/env perl

+use warnings;
 use strict;

 sub Usage {

lbartoletti avatar Aug 08 '22 08:08 lbartoletti

Sorry for the delay. I am on business trip. will merge now, and release will come out on the next weekend. Thanks for contributing.

cvvergara avatar Aug 21 '22 17:08 cvvergara