Skip to content
  • Categories
  • Recent
  • Tags
  • Popular
  • Users
  • Groups
  • Search
  • Get Qt Extensions
  • Unsolved
Collapse
Brand Logo
  1. Home
  2. General talk
  3. The Lounge
  4. Recurring C++ and Qt anti-patterns
Forum Updated to NodeBB v4.3 + New Features

Recurring C++ and Qt anti-patterns

Scheduled Pinned Locked Moved The Lounge
126 Posts 17 Posters 60.2k Views 10 Watching
  • Oldest to Newest
  • Newest to Oldest
  • Most Votes
Reply
  • Reply as topic
Log in to reply
This topic has been deleted. Only users with topic management privileges can see it.
  • aha_1980A Offline
    aha_1980A Offline
    aha_1980
    Lifetime Qt Champion
    wrote on last edited by
    #6

    I just saw the following again:

    auto *p = new ...;
    // ...
    if (p != NULL) {
      delete p;
      p = NULL;
    }
    

    which can be shortened to:

    delete p;
    p = NULL; // p = nullptr in C++11 and upwards
    

    because delete does the check anyway.

    Qt has to stay free or it will die.

    J.HilkJ fcarneyF 2 Replies Last reply
    4
    • aha_1980A aha_1980

      I just saw the following again:

      auto *p = new ...;
      // ...
      if (p != NULL) {
        delete p;
        p = NULL;
      }
      

      which can be shortened to:

      delete p;
      p = NULL; // p = nullptr in C++11 and upwards
      

      because delete does the check anyway.

      J.HilkJ Online
      J.HilkJ Online
      J.Hilk
      Moderators
      wrote on last edited by
      #7

      @aha_1980
      well, I would give the creator here the benefit of the doubt and they that's refactored code and used to be p->deleteLater() :-)


      Be aware of the Qt Code of Conduct, when posting : https://forum.qt.io/topic/113070/qt-code-of-conduct


      Q: What's that?
      A: It's blue light.
      Q: What does it do?
      A: It turns blue.

      aha_1980A 1 Reply Last reply
      2
      • J.HilkJ J.Hilk

        @aha_1980
        well, I would give the creator here the benefit of the doubt and they that's refactored code and used to be p->deleteLater() :-)

        aha_1980A Offline
        aha_1980A Offline
        aha_1980
        Lifetime Qt Champion
        wrote on last edited by
        #8

        Hi @J.Hilk,

        Yeah, but in that case it was pure C++ without Qt. Otherwise you would be right.

        Qt has to stay free or it will die.

        1 Reply Last reply
        0
        • aha_1980A aha_1980

          I just saw the following again:

          auto *p = new ...;
          // ...
          if (p != NULL) {
            delete p;
            p = NULL;
          }
          

          which can be shortened to:

          delete p;
          p = NULL; // p = nullptr in C++11 and upwards
          

          because delete does the check anyway.

          fcarneyF Offline
          fcarneyF Offline
          fcarney
          wrote on last edited by
          #9

          @aha_1980 said in Recurring C++ and Qt anti-patterns:

          because delete does the check anyway

          Wait, what? Since when does C++ check if the pointer is nullptr before delete? I didn't know this was a thing now.

          Also, is deleteLater not a way to delete Qt objects? Its morning and I haven't had my coffee.

          C++ is a perfectly valid school of magic.

          fcarneyF 1 Reply Last reply
          0
          • fcarneyF fcarney

            @aha_1980 said in Recurring C++ and Qt anti-patterns:

            because delete does the check anyway

            Wait, what? Since when does C++ check if the pointer is nullptr before delete? I didn't know this was a thing now.

            Also, is deleteLater not a way to delete Qt objects? Its morning and I haven't had my coffee.

            fcarneyF Offline
            fcarneyF Offline
            fcarney
            wrote on last edited by
            #10

            @fcarney said in Recurring C++ and Qt anti-patterns:

            Wait, what? Since when does C++ check if the pointer is nullptr before delete?

            Wow, since 2003? Lol, keep this thread going! I am learning a lot.

            C++ is a perfectly valid school of magic.

            fcarneyF 1 Reply Last reply
            1
            • fcarneyF fcarney

              @fcarney said in Recurring C++ and Qt anti-patterns:

              Wait, what? Since when does C++ check if the pointer is nullptr before delete?

              Wow, since 2003? Lol, keep this thread going! I am learning a lot.

              fcarneyF Offline
              fcarneyF Offline
              fcarney
              wrote on last edited by fcarney
              #11

              Okay, I am just confusing myself. If you delete a pointer you must immediately set it to null. Otherwise you risk double delete, which is bad. But its okay to delete something set to null. Got it.

              Edit:
              Why doesn't delete set the pointer to null then? That seems like it may be an antipattern in and of itself.

              C++ is a perfectly valid school of magic.

              aha_1980A Christian EhrlicherC 2 Replies Last reply
              1
              • fcarneyF fcarney

                Okay, I am just confusing myself. If you delete a pointer you must immediately set it to null. Otherwise you risk double delete, which is bad. But its okay to delete something set to null. Got it.

                Edit:
                Why doesn't delete set the pointer to null then? That seems like it may be an antipattern in and of itself.

                aha_1980A Offline
                aha_1980A Offline
                aha_1980
                Lifetime Qt Champion
                wrote on last edited by
                #12

                @fcarney said in Recurring C++ and Qt anti-patterns:

                Why doesn't delete set the pointer to null then? That seems like it may be an antipattern in and of itself.

                I have indeed asked that myself. If someone has the correct answer for that, I'm all ears.

                Qt has to stay free or it will die.

                fcarneyF ODБOïO S 3 Replies Last reply
                0
                • aha_1980A aha_1980

                  @fcarney said in Recurring C++ and Qt anti-patterns:

                  Why doesn't delete set the pointer to null then? That seems like it may be an antipattern in and of itself.

                  I have indeed asked that myself. If someone has the correct answer for that, I'm all ears.

                  fcarneyF Offline
                  fcarneyF Offline
                  fcarney
                  wrote on last edited by
                  #13

                  @aha_1980
                  Apparently the standard allows for it:
                  https://stackoverflow.com/questions/704466/why-doesnt-delete-set-the-pointer-to-null

                  The creator himself wonders why it isn't so. Its like C++ is this beautiful, amazing, and now, WILD animal roaming free in cyberspace... Yeah, maybe the analogy isn't all that great, but it does conjure up a cool picture.

                  C++ is a perfectly valid school of magic.

                  Kent-DorfmanK 1 Reply Last reply
                  1
                  • aha_1980A aha_1980

                    @fcarney said in Recurring C++ and Qt anti-patterns:

                    Why doesn't delete set the pointer to null then? That seems like it may be an antipattern in and of itself.

                    I have indeed asked that myself. If someone has the correct answer for that, I'm all ears.

                    ODБOïO Offline
                    ODБOïO Offline
                    ODБOï
                    wrote on last edited by
                    #14

                    @fcarney said in Recurring C++ and Qt anti-patterns:

                    Why doesn't delete set the pointer to null

                    likely because it would bring more problems than solutions

                    aha_1980A 1 Reply Last reply
                    0
                    • ODБOïO ODБOï

                      @fcarney said in Recurring C++ and Qt anti-patterns:

                      Why doesn't delete set the pointer to null

                      likely because it would bring more problems than solutions

                      aha_1980A Offline
                      aha_1980A Offline
                      aha_1980
                      Lifetime Qt Champion
                      wrote on last edited by
                      #15

                      @LeLev

                      likely because it would bring more problems than solutions

                      That would mean, that this pointer shows to an invalid memory region after the delete. Can you think of an example where you still want to use that pointer afterwards? (That is a real question - because for now I have no idea).

                      Qt has to stay free or it will die.

                      ODБOïO 1 Reply Last reply
                      0
                      • fcarneyF Offline
                        fcarneyF Offline
                        fcarney
                        wrote on last edited by
                        #16

                        @LeLev said in Recurring C++ and Qt anti-patterns:

                        likely because it would bring more problems than solutions

                        I could see a case where a program is deleting thousands of pointers and there might actually be overhead in a mov instruction for each delete. I have no idea if this overhead would be significant over the delete operation, but it would still be overhead. It would not be that hard to test such a scenario. I should try it!

                        C++ is a perfectly valid school of magic.

                        fcarneyF 1 Reply Last reply
                        0
                        • aha_1980A aha_1980

                          @LeLev

                          likely because it would bring more problems than solutions

                          That would mean, that this pointer shows to an invalid memory region after the delete. Can you think of an example where you still want to use that pointer afterwards? (That is a real question - because for now I have no idea).

                          ODБOïO Offline
                          ODБOïO Offline
                          ODБOï
                          wrote on last edited by
                          #17

                          @aha_1980 said in Recurring C++ and Qt anti-patterns:

                          Can you think of an example where you still want to use that pointer afterwards?

                          not a real world application

                          1 Reply Last reply
                          0
                          • fcarneyF fcarney

                            @LeLev said in Recurring C++ and Qt anti-patterns:

                            likely because it would bring more problems than solutions

                            I could see a case where a program is deleting thousands of pointers and there might actually be overhead in a mov instruction for each delete. I have no idea if this overhead would be significant over the delete operation, but it would still be overhead. It would not be that hard to test such a scenario. I should try it!

                            fcarneyF Offline
                            fcarneyF Offline
                            fcarney
                            wrote on last edited by
                            #18

                            @fcarney said in Recurring C++ and Qt anti-patterns:

                            deleting thousands of pointers and there might actually be overhead

                            I cannot actually tell if the overhead in this code is the indexing of the array, or if the movement of data is significant. I tried doing a dummy no op index, but I am guessing it is being optimized out:

                            #include <QCoreApplication>
                            #include <QElapsedTimer>
                            #include <QDebug>
                            
                            #define MEM_SEG_LEN 8
                            #define MEM_SEGS 100000000
                            
                            char** createMemoryList(){
                                char** list = new char*[MEM_SEGS];
                                for(int index=0; index<MEM_SEGS; index++){
                                    list[index]=new char[MEM_SEG_LEN];
                                }
                            
                                return list;
                            }
                            
                            void deleteMemoryList(char** list){
                                for(int index=0; index<MEM_SEGS; index++){
                                    delete list[index];
                                    list[index]; // can you force an index to occur?
                                }
                                delete list;
                            }
                            
                            void deleteMemoryListNull(char** list){
                                for(int index=0; index<MEM_SEGS; index++){
                                    delete list[index];
                                    list[index] = nullptr;
                                }
                                delete list;
                                list = nullptr;
                            }
                            
                            int main(int argc, char *argv[])
                            {
                                QCoreApplication a(argc, argv);
                            
                                QElapsedTimer timer1;
                            
                                char** list1 = createMemoryList();
                                timer1.start();
                                deleteMemoryList(list1);
                                qInfo() << timer1.elapsed();
                            
                                QElapsedTimer timer2;
                            
                                char** list2 = createMemoryList();
                                timer2.start();
                                deleteMemoryListNull(list2);
                                qInfo() << timer2.elapsed();
                            
                                return a.exec();
                            }
                            

                            I get the following output:

                            813
                            1301
                            

                            I doubt that is the overhead of the movement of null into the pointer. My guess is the the index overhead is in there too.

                            C++ is a perfectly valid school of magic.

                            1 Reply Last reply
                            0
                            • fcarneyF Offline
                              fcarneyF Offline
                              fcarney
                              wrote on last edited by fcarney
                              #19

                              I eliminated the extra index (probably compiler already did this before):

                              void deleteMemoryList(char** list){
                                  for(int index=0; index<MEM_SEGS; index++){
                                      char* tmp = list[index];
                                      delete tmp;
                                  }
                                  delete list;
                              }
                              
                              void deleteMemoryListNull(char** list){
                                  for(int index=0; index<MEM_SEGS; index++){
                                      char* tmp = list[index];
                                      delete tmp;
                                      tmp = nullptr;
                                  }
                                  delete list;
                                  list = nullptr;
                              }
                              

                              Results:

                              877
                              1369
                              

                              Edit: Real world usage? I really highly doubt it. That is a LOT of iterations of delete. So I would say the extra cycles are negligible.

                              Edit2:
                              Pointer math:

                              void deleteMemoryList(char** list){
                                  for(int index=0; index<MEM_SEGS; index++){
                                      char** tmp = &(list[index]);
                                      delete *tmp;
                                  }
                                  delete list;
                              }
                              
                              void deleteMemoryListNull(char** list){
                                  for(int index=0; index<MEM_SEGS; index++){
                                      char** tmp = &(list[index]);
                                      delete *tmp;
                                      *tmp = nullptr;
                                  }
                                  delete list;
                                  list = nullptr;
                              }
                              

                              Results:

                              853
                              1307
                              

                              Sometimes apples and apples is hard.

                              C++ is a perfectly valid school of magic.

                              JonBJ 1 Reply Last reply
                              0
                              • fcarneyF fcarney

                                I eliminated the extra index (probably compiler already did this before):

                                void deleteMemoryList(char** list){
                                    for(int index=0; index<MEM_SEGS; index++){
                                        char* tmp = list[index];
                                        delete tmp;
                                    }
                                    delete list;
                                }
                                
                                void deleteMemoryListNull(char** list){
                                    for(int index=0; index<MEM_SEGS; index++){
                                        char* tmp = list[index];
                                        delete tmp;
                                        tmp = nullptr;
                                    }
                                    delete list;
                                    list = nullptr;
                                }
                                

                                Results:

                                877
                                1369
                                

                                Edit: Real world usage? I really highly doubt it. That is a LOT of iterations of delete. So I would say the extra cycles are negligible.

                                Edit2:
                                Pointer math:

                                void deleteMemoryList(char** list){
                                    for(int index=0; index<MEM_SEGS; index++){
                                        char** tmp = &(list[index]);
                                        delete *tmp;
                                    }
                                    delete list;
                                }
                                
                                void deleteMemoryListNull(char** list){
                                    for(int index=0; index<MEM_SEGS; index++){
                                        char** tmp = &(list[index]);
                                        delete *tmp;
                                        *tmp = nullptr;
                                    }
                                    delete list;
                                    list = nullptr;
                                }
                                

                                Results:

                                853
                                1307
                                

                                Sometimes apples and apples is hard.

                                JonBJ Offline
                                JonBJ Offline
                                JonB
                                wrote on last edited by JonB
                                #20

                                @fcarney
                                Since this is the lounge... Surprised by your findings (in earlier examples). What exactly is the difference in the assembly between the two versions? What is being generated for your tmp = nullptr;? (Not the later *tmp = nullptr;, that's different.)

                                1 Reply Last reply
                                0
                                • fcarneyF Offline
                                  fcarneyF Offline
                                  fcarney
                                  wrote on last edited by
                                  #21

                                  @JonB said in Recurring C++ and Qt anti-patterns:

                                  tmp = nullptr;

                                  I changed it to not update a local variable. char* tmp is local, so setting it to null is just setting a local variable to null. So it was setting the wrong area of memory to null. That is why I took the address of where that pointer is stored.

                                  The assembler for tmp = nullptr in previous incarnation:

                                  movq   $0x0,-0x8(%rbp)
                                  

                                  The assembler for *tmp = nullptr in latest incarnation:

                                  mov    -0x8(%rbp),%rax
                                  movq   $0x0,(%rax)
                                  

                                  But you are right, the tmp = nullptr is more representative.
                                  The timing is not much different.

                                  C++ is a perfectly valid school of magic.

                                  JonBJ 1 Reply Last reply
                                  0
                                  • fcarneyF Offline
                                    fcarneyF Offline
                                    fcarney
                                    wrote on last edited by fcarney
                                    #22

                                    Okay, I think I am done, but here is my last incarnation:

                                    #include <QCoreApplication>
                                    #include <QElapsedTimer>
                                    #include <QDebug>
                                    
                                    #define MEM_SEG_LEN 8
                                    #define MEM_SEGS 100000000 // 846 1315 // assignment
                                    #define MEM_SEGS 100000000 // 885 1349 // correct assignment
                                    //#define MEM_SEGS 100
                                    
                                    char** createMemoryList(){
                                        char** list = new char*[MEM_SEGS];
                                        for(int index=0; index<MEM_SEGS; index++){
                                            list[index]=new char[MEM_SEG_LEN];
                                        }
                                    
                                        return list;
                                    }
                                    
                                    void deleteMemoryList(char** list){
                                        for(int index=0; index<MEM_SEGS; index++){
                                            char** tmp = &(list[index]);
                                            delete *tmp;
                                        }
                                        delete list;
                                    }
                                    
                                    void deleteMemoryListNull(char** list){
                                        for(int index=0; index<MEM_SEGS; index++){
                                            char** tmp = &(list[index]);
                                            delete *tmp;
                                            *tmp = nullptr;
                                    //        char* tmp = (list[index]);
                                    //        delete tmp;
                                    //        tmp = nullptr;
                                        }
                                        delete list;
                                        list = nullptr;
                                    }
                                    
                                    int main(int argc, char *argv[])
                                    {
                                        QCoreApplication a(argc, argv);
                                    
                                        double time1, time2;
                                    
                                        qInfo() << QString(R"(Starting memory test 1 of not setting pointer to nullptr: %1 cycles)").arg(MEM_SEGS);
                                    
                                        QElapsedTimer timer1;
                                    
                                        char** list1 = createMemoryList();
                                        timer1.start();
                                        deleteMemoryList(list1);
                                        time1 = timer1.elapsed();
                                        qInfo() << time1/1000.0 << QString().number((time1/1000.0)/double(MEM_SEGS),10,12);
                                    
                                        qInfo() << QString(R"(Starting memory test 2 of setting pointer to nullptr: %1 cycles)").arg(MEM_SEGS);
                                    
                                        QElapsedTimer timer2;
                                    
                                        char** list2 = createMemoryList();
                                        timer2.start();
                                        deleteMemoryListNull(list2);
                                        time2 = timer2.elapsed();
                                        qInfo() << time2/1000.0 << QString().number((time2/1000.0)/double(MEM_SEGS),10,12);
                                    
                                        qInfo() << "Difference:" << QString().number((time2/1000.0)/double(MEM_SEGS)-(time1/1000.0)/double(MEM_SEGS),10,12);
                                    
                                        return a.exec();
                                    }
                                    

                                    So, 5 nanoseconds difference for a delete operation of dereffed pointer assignment:

                                    "Starting memory test 1 of not setting pointer to nullptr: 100000000 cycles"
                                    0.848 "0.000000008480"
                                    "Starting memory test 2 of setting pointer to nullptr: 100000000 cycles"
                                    1.321 "0.000000013210"
                                    Difference: "0.000000004730"
                                    

                                    Edit:
                                    Math was wrong.

                                    C++ is a perfectly valid school of magic.

                                    1 Reply Last reply
                                    0
                                    • fcarneyF fcarney

                                      @JonB said in Recurring C++ and Qt anti-patterns:

                                      tmp = nullptr;

                                      I changed it to not update a local variable. char* tmp is local, so setting it to null is just setting a local variable to null. So it was setting the wrong area of memory to null. That is why I took the address of where that pointer is stored.

                                      The assembler for tmp = nullptr in previous incarnation:

                                      movq   $0x0,-0x8(%rbp)
                                      

                                      The assembler for *tmp = nullptr in latest incarnation:

                                      mov    -0x8(%rbp),%rax
                                      movq   $0x0,(%rax)
                                      

                                      But you are right, the tmp = nullptr is more representative.
                                      The timing is not much different.

                                      JonBJ Offline
                                      JonBJ Offline
                                      JonB
                                      wrote on last edited by JonB
                                      #23

                                      @fcarney
                                      You're talking about a single movq. Just how big is MEM_SEGS? Your timings, are they in milliseconds?? And I assume list is filled with zeroes? I'm talking about your earlier barebones example, where your timings were
                                      877
                                      1369

                                      Oh now I see more code in other examples. If your list contained 10 million news and you are deleteing them, common-sense should tell you the cost of whether or not you set one local variable to nullptr with just a mov instruction must be negligible, compared to whatever is involved in freeing memory, no?

                                      1 Reply Last reply
                                      0
                                      • fcarneyF Offline
                                        fcarneyF Offline
                                        fcarney
                                        wrote on last edited by
                                        #24

                                        @JonB said in Recurring C++ and Qt anti-patterns:

                                        one local variable to nullptr with just a mov instruction must be negligible, compared to whatever is involved in freeing memory, no?

                                        Correct. My math was wrong. It is 5 nanoseconds. At least for dereffed pointer, but timing was nearly the same for local variable as well.

                                        C++ is a perfectly valid school of magic.

                                        JonBJ 1 Reply Last reply
                                        3
                                        • fcarneyF fcarney

                                          @JonB said in Recurring C++ and Qt anti-patterns:

                                          one local variable to nullptr with just a mov instruction must be negligible, compared to whatever is involved in freeing memory, no?

                                          Correct. My math was wrong. It is 5 nanoseconds. At least for dereffed pointer, but timing was nearly the same for local variable as well.

                                          JonBJ Offline
                                          JonBJ Offline
                                          JonB
                                          wrote on last edited by
                                          #25

                                          @fcarney
                                          Ahhh, that would make much more sense! :)
                                          A nanosecond doesn't sound too long, I don't think I could get much done in it could I?

                                          1 Reply Last reply
                                          0

                                          • Login

                                          • Login or register to search.
                                          • First post
                                            Last post
                                          0
                                          • Categories
                                          • Recent
                                          • Tags
                                          • Popular
                                          • Users
                                          • Groups
                                          • Search
                                          • Get Qt Extensions
                                          • Unsolved