Fix more performance issues found by cppcheck#51

Closed

dalgaaf wants to merge 61 commits intoceph:masterfrom

dalgaaf:wip-da-sca-cppcheck-performance-2

Contributor

dalgaaf commented Feb 13, 2013

Here some more patches to fix performance issues found by cppcheck. This should now cover - together with wip-da-sca-cppcheck-performance - the following issues:

use empty() instead of size() to check for emptiness
dont't pass string::c_str() to string arguments
prevent useless value assignment
pass some objects by reference instead of by-value

dalgaaf added 30 commits

February 13, 2013 13:59


          CephxProtocol.h: pass CryptoKey by reference to decode_decrypt()

a3970e5

Fix issues found by cppcheck:

[src/auth/cephx/CephxProtocol.h:469]: (performance) Function
  parameter 'key' should be passed by reference.

Pass CryptoKey by reference as already done in encode_encrypt().

Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>


          CInode.h: use !old_inodes.empty() instead of size()

6b8458d

Use empty() since it should be prefered as it has, following the
standard, a constant time complexity regardless of the containter
type. The same is not guaranteed for size().

warning from cppcheck was:
[src/mds/journal.cc:470]: (performance) Possible inefficient
  checking for 'old_inodes' emptiness.

Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>


          AuthMonitor.cc: use !pending_auth.empty() instead of 'size() > 0'

180cae5

Use empty() since it should be prefered as it has, following the
standard, a constant time complexity regardless of the containter
type. The same is not guaranteed for size().

warning from cppcheck was:
[src/mon/AuthMonitor.cc:210]: (performance) Possible inefficient
  checking for 'pending_auth' emptiness.

Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>


          OSDMonitor.h: use !reporters.empty() instead of size()

d7705ed

Use empty() since it should be prefered as it has, following the
standard, a constant time complexity regardless of the containter
type. The same is not guaranteed for size().

Using empty() in this case makes the code more logical.

warning from cppcheck was:

[src/mon/OSDMonitor.h:59]: (performance) Possible inefficient
  checking for 'reporters' emptiness.

Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>


          MonCaps.cc: use !empty() instead of size()

f7078b4

Use empty() since it should be prefered as it has, following the
standard, a constant time complexity regardless of the containter
type. The same is not guaranteed for size().

Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>


          Monitor.cc: use empty() instead of size()

8fcd31f

Use empty() since it should be prefered as it has, following the
standard, a constant time complexity regardless of the containter
type. The same is not guaranteed for size().

warning from cppcheck was:
[src/mon/Monitor.cc:487]: (performance) Possible inefficient
  checking for 'initial_members' emptiness.
[src/mon/Monitor.cc:1361]: (performance) Possible inefficient
  checking for 'timecheck_skews' emptiness.
[src/mon/Monitor.cc:2302]: (performance) Possible inefficient
  checking for 'timecheck_waiting' emptiness.
[src/mon/Monitor.cc:2547]: (performance) Possible inefficient
  checking for 'timecheck_waiting' emptiness.

Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>


          OSDMonitor.cc: use !empty() instead of size()

6f0175e

Use empty() since it should be prefered as it has, following the
standard, a constant time complexity regardless of the containter
type. The same is not guaranteed for size().

Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>


          PGMonitor.cc: use !empty() instead of size() to check for emptiness

0b31234

Use empty() since it should be prefered as it has, following the
standard, a constant time complexity regardless of the containter
type. The same is not guaranteed for size().

Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>


          monmaptool.cc: use empty() instead of size() to check for emptiness

dd855b3

Use empty() since it should be prefered as it has, following the
standard, a constant time complexity regardless of the containter
type. The same is not guaranteed for size().

Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>


          DBObjectMap.cc: use empty() instead of size() to check for emptiness

b0ec071

Use empty() since it should be prefered as it has, following the
standard, a constant time complexity regardless of the containter
type. The same is not guaranteed for size().

Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>


          FileStore.cc: use empty() instead of size() to check for emptiness

f65679b

Use empty() since it should be prefered as it has, following the
standard, a constant time complexity regardless of the containter
type. The same is not guaranteed for size().

Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>


          HashIndex.cc: use empty() instead of size() to check for emptiness

4893def

Use empty() since it should be prefered as it has, following the
standard, a constant time complexity regardless of the containter
type. The same is not guaranteed for size().

Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>


          LFNIndex.cc: use !holes.empty() instead of 'size() > 0'

f94f797

Use empty() since it should be prefered as it has, following the
standard, a constant time complexity regardless of the containter
type. The same is not guaranteed for size().

Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>


          OSD.cc: use empty() instead of size() to check for emptiness

5d89996

Use empty() since it should be prefered as it has, following the
standard, a constant time complexity regardless of the containter
type. The same is not guaranteed for size().

This replaces also some size() >=/== checks.

Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>


          PG.cc: use empty() instead of size() to check for emptiness

d9a0638

Use empty() since it should be prefered as it has, following the
standard, a constant time complexity regardless of the containter
type. The same is not guaranteed for size().

This replaces also some size() >=/== checks.

Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>


          ReplicatedPG.cc: use empty() instead of size() to check for emptiness

475967f

Use empty() since it should be prefered as it has, following the
standard, a constant time complexity regardless of the containter
type. The same is not guaranteed for size().

This replaces also some size() >=/== checks.

Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>


          ObjectCacher.cc: use empty() instead of !size() to check for emptiness

722d12c

Use empty() since it should be prefered as it has, following the
standard, a constant time complexity regardless of the containter
type. The same is not guaranteed for size().

Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>


          Objecter.cc: use !empty() instead of size() to check for emptiness

612f65b

Use empty() since it should be prefered as it has, following the
standard, a constant time complexity regardless of the containter
type. The same is not guaranteed for size().

Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>


          Objecter.cc: prevent useless value assignment

3fc14b3

Fix issue found by cppcheck:
[src/osdc/Objecter.cc:989] -> [src/osdc/Objecter.cc:992]: (performance)
  Variable 'check_for_latest_map' is reassigned a value before the old
  one has been used.

Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>


          osdmaptool.cc: : use empty() instead of 'size() < 1'

7e13421

Use empty() since it should be prefered as it has, following the
standard, a constant time complexity regardless of the containter
type. The same is not guaranteed for size().

This way the code is more logical.

Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>


          rados.cc: use omap.empty() instead of size() to check for emptiness

1b241b1

Use empty() since it should be prefered as it has, following the
standard, a constant time complexity regardless of the containter
type. The same is not guaranteed for size().

Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>


          rbd.cc: use empty() instead of size() to check for emptiness

Use empty() since it should be prefered as it has, following the
standard, a constant time complexity regardless of the containter
type. The same is not guaranteed for size().

Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>


          rgw/rgw_admin.cc: prevent useless value assignment

7ca6e5d

Fix issue found by cppcheck:
[src/rgw/rgw_admin.cc:710] -> [src/rgw/rgw_admin.cc:714]:
  (performance) Variable 'ret' is reassigned a value before
  the old one has been used.

Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>


          rgw/rgw_admin.cc: use empty() instead of size() to check for emptiness

f087852

Use empty() since it should be prefered as it has, following the
standard, a constant time complexity regardless of the containter
type. The same is not guaranteed for size().

Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>


          rgw/rgw_gc.cc: use !empty() instead of size() to check for emptiness

92bef51

Use empty() since it should be prefered as it has, following the
standard, a constant time complexity regardless of the containter
type. The same is not guaranteed for size().

Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>


          rgw/rgw_log.cc: don't pass c_str() result to std::string argument

bdeb066

Fix issue found by cppcheck:

[src/rgw/rgw_log.cc:340]: (performance) Passing the result of c_str() to a
  function that takes std::string as argument no. 4 is slow and redundant.

Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>


          cls/rbd/cls_rbd.cc: use !empty() instead of 'size() > 0'

9178cf4

Use empty() since it should be prefered as it has, following the
standard, a constant time complexity regardless of the containter
type. The same is not guaranteed for size().

Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>


          cls_refcount.cc: use empty() instead of !size() to check for emptiness

268f208

Use empty() since it should be prefered as it has, following the
standard, a constant time complexity regardless of the containter
type. The same is not guaranteed for size().

Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>


          common/WorkQueue.cc: use !empty() instead of size() to check for empt…

083c43e

…iness

Use empty() since it should be prefered as it has, following the
standard, a constant time complexity regardless of the containter
type. The same is not guaranteed for size().

Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>


          obj_bencher.cc: use empty() instead of 'size() == 0' to check for emp…

40cc3df

…tiness

Use empty() since it should be prefered as it has, following the
standard, a constant time complexity regardless of the containter
type. The same is not guaranteed for size().

Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>

dalgaaf added 15 commits

February 13, 2013 18:40


          rgw/rgw_rados.cc: : use empty() to check for emptiness

a955d12

Use empty() instead of '!size()' for performance reasons.

Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>


          rgw/rgw_rest_swift.cc: don't pass c_str() result to std::string argument

8fbe3e4

Fix issue found by cppcheck:
[src/rgw/rgw_rest_swift.cc:770]: (performance) Passing the result of
  c_str() to a function that takes std::string as argument no. 1 is
  slow and redundant.

Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>


          rgw/rgw_usage.cc use empty() to check for emptiness

512bc4f

Use !empty() instead of 'size()' to check for emptiness for
performance reasons.

Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>


          rgw/rgw_user.cc: use empty() to check for emptiness

661ec95

Use !empty() instead of 'size()' to check for emptiness for
performance reasons.

Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>


          scratchtoolpp.cc: print some more results

5a9482b

Print some more results to fix issue found by cppcheck:

[src/scratchtoolpp.cc:111] -> [src/scratchtoolpp.cc:114]:
  (performance) Variable 'r' is reassigned a value before
  the old one has been used.
[src/scratchtoolpp.cc:207] -> [src/scratchtoolpp.cc:208]:
  (performance) Variable 'r' is reassigned a value before
  the old one has been used.

Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>


          test_keyvaluedb_iterators.cc: use empty() to check for emptiness

339fc4c

Use !empty() instead of 'size() > 0' to check for emptiness for
performance reasons.

Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>


          test_object_map.cc: use empty() to check for emptiness

10eacbd

Use !empty() instead of 'size()' to check for emptiness for
performance reasons.

Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>


          FileStoreTracker.cc: use empty() to check for emptiness

5a516ef

Use !empty() instead of size() to check for emptiness for
performance reasons.

Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>


          TestFileStoreState.cc: use empty() to check for emptiness

9d7d887

Use empty() instead of '!size()' to check for emptiness for
performance reasons.

Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>


          test_mon_workloadgen.cc: use empty() to check for emptiness

948bd89

Use empty() instead of '!size()' and "size() == 0" to check for emptiness
for performance reasons and since it's more logical.

Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>


          src/test/osd/RadosModel.h: use empty() to check for emptiness

8f037e1

Use empty() instead of size() to check for emptiness for
performance reasons.

Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>


          test_filejournal.cc: use empty() to check for emptiness

c4f269e

Use !empty() instead of size() to check for emptiness for
performance reasons.

Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>


          tools/common.cc. use !empty() to check for non-emptiness

67a56e5

Use !empty() instead of size() to check for non-emptiness for
performance reasons.

Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>


          rest_bench.cc: use empty() to check for emptiness

e19d0bf

Use empty() instead of 'size() == 0' or 'size() < 1' to check
for emptiness for performance reasons.

Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>


          wireshark/ceph/packet-ceph.c: fix some issues from cppcheck

fc85400

Fix some smaller performance related issues from cppcheck due to
"Variable '..xy..' is reassigned a value before the old one has been used.

Fix some indention.

Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>

Member

gregsfortytwo commented Feb 14, 2013

Merged with a few modifications as of ffda2ea. More discussion on the mailing list.

gregsfortytwo closed this

Contributor Author

dalgaaf commented Feb 14, 2013

@gregsfortytwo: Thanks. AFAICS everything is in except the gtest patch. Correct me if I'm wrong.

gregsfortytwo mentioned this pull request

Fix some more issues found with cppcheck #48

Closed

XinzeChi pushed a commit to XinzeChi/ceph that referenced this pull request


          Merge pull request ceph#51 from XinzeChi/mslovy-wip-findObjectContext…

d882d9d

…-opt

ReplicatedPG:: find_object_context clear extra creation (head,snapdir)

ddiss pushed a commit to ddiss/ceph that referenced this pull request


          Merge pull request ceph#51 from kmroz/wip-port-civetweb-https-fix

dada3f3

port civetweb https fix

Reviewed-by: Nathan Cutler <ncutler@suse.com>

ricardoasmarques added a commit to ricardoasmarques/ceph that referenced this pull request


          Merge pull request ceph#51 from rjfd/wip-oa-mgr-fix-authrequired

88b038d

mgr/dashboard_v2: Fix bug in AuthRequired decorator

dotnwat added a commit that referenced this pull request


          Merge pull request #51 from nanjekyejoannah/add_go_vet

8b9635f

Add go vet to Travis

lixiaoy1 added a commit to lixiaoy1/ceph that referenced this pull request


          Merge branch '51-performance-degraded-on-branch-wip-librbd-rwl-3' int…

4f01e20

…o 'wip-librbd-rwl-3'

Resolve "Performance degraded on branch wip-librbd-rwl-3"

Closes ceph#51

See merge request sdpeters/ceph-rwl!33

snyk-bot mentioned this pull request

[Snyk] Security upgrade stylelint-declaration-use-variable from 1.7.2 to 1.7.3 ekmixon/ceph#10

Open

snyk-bot mentioned this pull request

[Snyk] Security upgrade stylelint-declaration-use-variable from 1.7.2 to 1.7.3 ekmixon/ceph#14

Open

majianpeng mentioned this pull request

qa/suites/rbd: place cache file on tmpfs for xfstests #46045

Merged

14 tasks

irq0 pushed a commit to irq0/ceph that referenced this pull request


          Merge pull request ceph#51 from jecluis/wip-sfs-fix-users-sqlite

4ead447

rgw/sfs: ensure sqlite user ops hold required locks

tobias-urdin pushed a commit to tobias-urdin/ceph that referenced this pull request


          Merge pull request ceph#51 from andrewgaul/nuke-bucket-without-versions

e4dafa0

Fall back when nuking buckets

Reviewed-by: Yehuda Sadeh <yehuda@redhat.com>

athanatos pushed a commit to athanatos/ceph that referenced this pull request


          Merge pull request ceph#51 from cbodley/wip-tag-delay

0a11649

server: delayed tag calculation as a template parameter

Reviewed-by: J. Eric Ivancich <ivancich@redhat.com>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet