osd/OSD.h: change iterator to const_iterator

[stiopaa1]

Signed-off-by: Michal Jarzabek stiopa@gmail.com

[tchaikov]

this creates a const_iterator from an iterator. which incurs the cost of calling a copy ctor. if you want to make sure that we are using the const_iterator for reading containers. maybe you could use auto, session_waiting_for_pg.cbegin(), session_waiting_for_pg.cend() instead?

[stiopaa1]

@tchaikov
I have changed the code to return const_iterators where possible. In the cases where it's not possible to return const_iterator I think it's still worth to use them. It's just a conversion from iterator to const_iterator, which might even be optimised out by the compiler.

[tchaikov]

It's just a conversion from iterator to const_iterator,

yes, it creates an iterator then calls the ctor of const_iterator, then throw the original iterator away.

which might even be optimised out by the compiler.

i am not sure about this though.

[stiopaa1]

@tchaikov

i am not sure about this though.

I wrote this program to check:

#include <set>
#include <iostream>


std::set<int> aa { 1, 2 };
void func1()
{
  std::set<int>::iterator it = aa.find(1);
  std::cout << *it;
}
void func2()
{
  std::set<int>::const_iterator it = aa.find(1);
  std::cout << *it;
}
int main()
{
  func1();
  func2();
}

stiopa@stiopa-desktop|~/temp/constiter_opt
) g++ test.cc -std=c++11 -S -O3

stiopa@stiopa-desktop|~/temp/constiter_opt
) diff ff1 ff2
diff --git a/ff1 b/ff2
index 94099ec..55641a0 100644
--- a/ff1
+++ b/ff2
@@ -1,36 +1,36 @@

-_Z5func1v:
-.LFB1572:
+_Z5func2v:
+.LFB1573:
        .cfi_startproc
        movq    aa+16(%rip), %rax
        movl    $aa+8, %edx
        testq   %rax, %rax
-       jne     .L10
-       jmp     .L7
+       jne     .L25
+       jmp     .L22
        .p2align 4,,10
        .p2align 3
-.L15:
+.L29:
        movq    %rax, %rdx
        movq    16(%rax), %rax
        testq   %rax, %rax
-       je      .L14
-.L10:
+       je      .L28
+.L25:
        movl    32(%rax), %ecx
        testl   %ecx, %ecx
-       jg      .L15
+       jg      .L29
        movq    24(%rax), %rax
        testq   %rax, %rax
-       jne     .L10
-.L14:
+       jne     .L25
+.L28:
        cmpq    $aa+8, %rdx
-       je      .L7
+       je      .L22
        movl    32(%rdx), %esi
        cmpl    $1, %esi
-       jg      .L7
+       jg      .L22
        movl    $_ZSt4cout, %edi
        jmp     _ZNSolsEi
        .p2align 4,,10
        .p2align 3
-.L7:
+.L22:
        movl    aa+40(%rip), %esi
        movl    $_ZSt4cout, %edi
        jmp     _ZNSolsEi

I don't have much experience with assembly code, so this test might be completely wrong, but if it's not then the both functions look the same?

[tchaikov]

@stiopaa1 you are quite right! std::set::iterator is an alias of std::set::const_iterator since C++11. and i changed your test a little bit to see how the compiler optimizes the map::iterator with -O2, it turns out that the gcc simply strips the cover of const_iterator and just looks at its innards, and yes, the ctor of const_iterator is not called at all. this is quite different from what we have with -O0.

[tchaikov]

lgtm

[tchaikov]

tested at

• http://pulpito.ceph.com/kchai-2016-06-10_23:14:31-rados-wip-kefu-testing---basic-smithi/
• http://pulpito.ceph.com/kchai-2016-06-07_23:04:35-rados-wip-kefu-testing---basic-mira/

failed test filed as http://tracker.ceph.com/issues/16239