Skip to content
  • Categories
  • Recent
  • Tags
  • Popular
  • Users
  • Groups
  • Search
  • Get Qt Extensions
  • Unsolved
Collapse
Brand Logo
  1. Home
  2. Qt Development
  3. General and Desktop
  4. Threads problems
QtWS25 Last Chance

Threads problems

Scheduled Pinned Locked Moved Solved General and Desktop
16 Posts 2 Posters 1.4k Views
  • 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.
  • D Offline
    D Offline
    DoubleC122
    wrote on last edited by
    #1

    To give a bit of context, I'm working on a small project, a zip/unzip tool, that is. Everything works fine except when I'm trying zip or unzip a file/folder with considerable size. Now, the problem is not the fact that it's taking quite a long time to zip/unzip, that is normal and I have also tested it with the zip tool I have installed (Ark) and it takes the same amount of time. The issue is that in my case, while it zips/unzips that file, my gui freezes and my general system performance suffers a bit. The first thing I thought to do was to use threads, of course.

    I tried with QThread in an isolated case with the unzip function first with both threads and no threads, and of course the threaded one does not put a strain on performance. The inconvenient is that it doesn't fully process the whole function, as it only extracts a few files (sometimes none) and then exits, which leaves me a bit baffled. Here is the whole unzip function and main:

    int copy_data(struct archive *ar, struct archive *aw)
    {
        int r;
        const void *buff;
        size_t size;
        la_int64_t offset;
    
        for (;;) {
    
            r = archive_read_data_block(ar, &buff, &size, &offset);
    
            if (r == ARCHIVE_EOF)
                return (ARCHIVE_OK);
    
            if (r < ARCHIVE_OK)
                return (r);
    
            r = archive_write_data_block(aw, buff, size, offset);
    
            if (r < ARCHIVE_OK) {
    
                fprintf(stderr, "%s\n", archive_error_string(aw));
    
                return (r);
            }
        }
    }
    
    void unzip_v2(const char* filename)
    {
        std::cout<<"ZIPNAME: "<<filename<<"\n";
    
        struct archive *a;
        struct archive *ext;
        struct archive_entry *entry;
        int flags;
        int r;
    
    
        /* Select which attributes we want to restore. */
        flags = ARCHIVE_EXTRACT_TIME;
        flags |= ARCHIVE_EXTRACT_PERM;
        flags |= ARCHIVE_EXTRACT_ACL;
        flags |= ARCHIVE_EXTRACT_FFLAGS;
    
        a = archive_read_new();
        archive_read_support_format_all(a);
        archive_read_support_compression_all(a);
        ext = archive_write_disk_new();
        archive_write_disk_set_options(ext, flags);
        archive_write_disk_set_standard_lookup(ext);
    
        if ((r = archive_read_open_filename(a, filename, 10240)))
        {
            std::cout<<"Could not open archive to read\n\n";
            return;
        }
    
        for (;;)
        {
            std::cout << "Number of threads = "
                      <<  std::thread::hardware_concurrency() << std::endl;
    
            r = archive_read_next_header(a, &entry);
    
            if (r == ARCHIVE_EOF)
                break;
    
            if (r < ARCHIVE_OK)
            {
                std::cout<<"An error occurred while reading from the archive\n\n";
                return;
            }
    
    
            if (r < ARCHIVE_WARN)
            {
                std::cout<<"An error occurred while reading from the archive\n\n";
                return;
            }
    
            r = archive_write_header(ext, entry);
    
            if (r < ARCHIVE_OK)
            {
                std::cout<<"An error occurred while reading from the archive\n\n";
    
            }
    
            else if (archive_entry_size(entry) > 0)
            {
    
                r = copy_data(a, ext);
    
                if (r < ARCHIVE_OK)
                {
                    std::cout<<"Could not copy data to archive\n\n";
                    return;
                }
    
                if (r < ARCHIVE_WARN)
                {
                    std::cout<<"Could not copy data to archive\n\n";
                    return;
                }
            }
    
            r = archive_write_finish_entry(ext);
    
            if (r < ARCHIVE_OK)
            {
                std::cout<<"Could not finish writing to archive\n\n";
                return;
            }
    
    
            if (r < ARCHIVE_WARN)
            {
                std::cout<<"Could not finish writing to archive\n\n";
                return;
            }
    
        }
    
        if(archive_read_close(a) != 0 || archive_read_free(a) != 0 || archive_write_close(ext) != 0 || archive_write_free(ext) != 0)
        {
            std::cout<<"Could not close archive\n\n";
            return;
        }
    }
    
    int main()
    {
        chdir("/home/user/Stuff/Test2/");
        const char* filename = "Test.zip";
    
        //unzip_v2(filename);
    
        QThread* thread = QThread::create(&unzip_v2, filename);
        thread->start();
    
        //    std::thread thread(&unzip_v2, filename);
        //    thread.join();
    
        return 0;
    }
    

    I also tried to use std::thread but doesn't seem to improve anything at all. The library I'm using for the unzip function is libarchive. Maybe I'm using threads the wrong way? I'd have to confess it's the first time I'm actually using them.

    1 Reply Last reply
    0
    • VRoninV Offline
      VRoninV Offline
      VRonin
      wrote on last edited by
      #2

      Replace

      @DoubleC122 said in Threads problems:

      QThread* thread = QThread::create(&unzip_v2, filename);
      thread->start();

      with std::async(std::launch::async,&unzip_v2,filename).get(); no need to use Qt for something std does probably better already.

      Your problem is that you are not waiting for the thread to finish, that shouldn't be a real problem as you mention a GUI in your question so the event look should be able to keep the program alive while the thread is running. you just need to make sure you join all threads (here I do it with the final .get() before the main finishes

      "La mort n'est rien, mais vivre vaincu et sans gloire, c'est mourir tous les jours"
      ~Napoleon Bonaparte

      On a crusade to banish setIndexWidget() from the holy land of Qt

      1 Reply Last reply
      2
      • D Offline
        D Offline
        DoubleC122
        wrote on last edited by
        #3

        Indeed, by using std::async I can definitely feel the performance is improved. One inconvenient is that the gui keeps freezing. By freezing I mean just what the gui displays, not that becomes completely unresponsive and then the program crashes. The program just keeps going while I'm unable to do other things inside the gui. Is that a problem that comes within Qt itself? It's quite strange to me.

        1 Reply Last reply
        0
        • VRoninV Offline
          VRoninV Offline
          VRonin
          wrote on last edited by VRonin
          #4

          Are you calling get() on the future returned by std::async? that's what freezes the GUI.

          You can change it to:

          QObject* finishObject= new QObject;
          QObject::connect(finishObject,&QObject::destroyed,[](){std::cout << "unzip_v2 finished";});
          std::async(std::launch::async,[finishObject](QByteArray filename){unzip_v2(filename.constData()); delete finishObject;},filename);
          

          So that your GUI does not stop and you have control on when the process actually finished.
          I used QByteArray so that you don't have to worry about the lifetime of the argument passed to unzip_v2

          One last note: never change your GUI directly from a secondary thread

          "La mort n'est rien, mais vivre vaincu et sans gloire, c'est mourir tous les jours"
          ~Napoleon Bonaparte

          On a crusade to banish setIndexWidget() from the holy land of Qt

          1 Reply Last reply
          2
          • D Offline
            D Offline
            DoubleC122
            wrote on last edited by
            #5

            Hm, doesn't seem to work. My unzip function is called from a signal function though, could that mean something related to the gui freezing?

            1 Reply Last reply
            0
            • VRoninV Offline
              VRoninV Offline
              VRonin
              wrote on last edited by
              #6

              The code above is GUI-less so it's hard to guess without knowing what your code looks like

              "La mort n'est rien, mais vivre vaincu et sans gloire, c'est mourir tous les jours"
              ~Napoleon Bonaparte

              On a crusade to banish setIndexWidget() from the holy land of Qt

              1 Reply Last reply
              0
              • D Offline
                D Offline
                DoubleC122
                wrote on last edited by DoubleC122
                #7

                Well it works like this. First, I have a signal called on_listView_customContextMenuRequested(const QPoint &pos) which will pop a context menu on right click. Then, based on what I select, I'll call the function that actually pops the menu, showContextMenu(pos). There, I assemble my menu with things like: myMenu.addAction("Unzip here") and other options. I guess it's worth to mention that I use QSignalMapper to connect some slots that have arguments (yes, I know it's deprecated, but I found that this way worked fine for me, might this actually be the issue? :/ ) and then connect everything like this:

                connect(action, SIGNAL(triggered(bool)), mapper, SLOT(map()));
                connect(mapper, SIGNAL(mapped(QString)), this, SLOT(unzip(QString)));
                mapper->setMapping(action, Fpath);
                

                And finally, in the unzip slot I call the unzip_v2 function. If there is the need of more explicit code, I'll post it, didn't want to make this reply too long. And also in the unzip slot is where I use the std::async.

                1 Reply Last reply
                0
                • VRoninV Offline
                  VRoninV Offline
                  VRonin
                  wrote on last edited by VRonin
                  #8

                  What I think it's happening is that the QString argument that you pass to unzip_v2 as const char* goes out of scope before unzip_v2 completed. could you show us the content of unzip?

                  P.S.
                  QSignalMapper is not the problem here. If you really want to get rid of it you can replace

                  connect(action, SIGNAL(triggered(bool)), mapper, SLOT(map()));
                  connect(mapper, SIGNAL(mapped(QString)), this, SLOT(unzip(QString)));
                  mapper->setMapping(action, Fpath);
                  

                  with connect(action,&QAction::triggered,this,std::bind(&MyClass::unzip,this,Fpath)); (where MyClass is the type of *this)

                  "La mort n'est rien, mais vivre vaincu et sans gloire, c'est mourir tous les jours"
                  ~Napoleon Bonaparte

                  On a crusade to banish setIndexWidget() from the holy land of Qt

                  VRoninV 1 Reply Last reply
                  1
                  • D Offline
                    D Offline
                    DoubleC122
                    wrote on last edited by DoubleC122
                    #9

                    Here is unzip:

                    void MainWindow::unzip(QString filePath)
                    {
                    	
                        boost::filesystem::path tempP(filePath.toStdString());
                        chdir(tempP.parent_path().string().c_str());
                        std::async(std::launch::async,&zipUtils::unzip_v2, tempP.filename().string().c_str()).get();
                    }
                    
                    1 Reply Last reply
                    0
                    • VRoninV VRonin

                      What I think it's happening is that the QString argument that you pass to unzip_v2 as const char* goes out of scope before unzip_v2 completed. could you show us the content of unzip?

                      P.S.
                      QSignalMapper is not the problem here. If you really want to get rid of it you can replace

                      connect(action, SIGNAL(triggered(bool)), mapper, SLOT(map()));
                      connect(mapper, SIGNAL(mapped(QString)), this, SLOT(unzip(QString)));
                      mapper->setMapping(action, Fpath);
                      

                      with connect(action,&QAction::triggered,this,std::bind(&MyClass::unzip,this,Fpath)); (where MyClass is the type of *this)

                      VRoninV Offline
                      VRoninV Offline
                      VRonin
                      wrote on last edited by VRonin
                      #10

                      Does this code even compile? is zipUtils a namespace?

                      Try changing std::async(std::launch::async,&zipUtils::unzip_v2, tempP.filename().string().c_str()).get();
                      to std::async(std::launch::async,[](std::string nameString){zipUtils::unzip_v2(nameString.c_str());}, tempP.filename().string());

                      "La mort n'est rien, mais vivre vaincu et sans gloire, c'est mourir tous les jours"
                      ~Napoleon Bonaparte

                      On a crusade to banish setIndexWidget() from the holy land of Qt

                      1 Reply Last reply
                      0
                      • D Offline
                        D Offline
                        DoubleC122
                        wrote on last edited by
                        #11

                        Yes, it does compile just fine and indeed, zipUtils is just a namespace. Even after changing those, it still continues to freeze. This problem seems to keep bugging me for a while now.

                        VRoninV 1 Reply Last reply
                        0
                        • D DoubleC122

                          Yes, it does compile just fine and indeed, zipUtils is just a namespace. Even after changing those, it still continues to freeze. This problem seems to keep bugging me for a while now.

                          VRoninV Offline
                          VRoninV Offline
                          VRonin
                          wrote on last edited by
                          #12

                          @DoubleC122 said in Threads problems:

                          it still continues to freeze.

                          Did you notice I'm not calling get() at the end? that's the prime suspect for the freeze, did you remove it as well?

                          "La mort n'est rien, mais vivre vaincu et sans gloire, c'est mourir tous les jours"
                          ~Napoleon Bonaparte

                          On a crusade to banish setIndexWidget() from the holy land of Qt

                          1 Reply Last reply
                          0
                          • D Offline
                            D Offline
                            DoubleC122
                            wrote on last edited by
                            #13

                            I actually added it at the end since I noticed it wasn't there.

                            VRoninV 1 Reply Last reply
                            0
                            • D DoubleC122

                              I actually added it at the end since I noticed it wasn't there.

                              VRoninV Offline
                              VRoninV Offline
                              VRonin
                              wrote on last edited by VRonin
                              #14

                              Yep, remove it. the fact that was missing was intentional.

                              To explain a bit more, I'm using a lambda here to prevent the data pointed by c_str() going out of scope

                              "La mort n'est rien, mais vivre vaincu et sans gloire, c'est mourir tous les jours"
                              ~Napoleon Bonaparte

                              On a crusade to banish setIndexWidget() from the holy land of Qt

                              1 Reply Last reply
                              0
                              • D Offline
                                D Offline
                                DoubleC122
                                wrote on last edited by
                                #15

                                I tested it right now once again but still won't unfreeze. I know that that for(;;) in unzip_v2 is quite heavy to compute with a large zip but that is why I thought it would get better with threads.

                                1 Reply Last reply
                                0
                                • D Offline
                                  D Offline
                                  DoubleC122
                                  wrote on last edited by
                                  #16

                                  I guess I found out a way to do this. It was just by chance, but, based on what you wrote here, I thought I'd try and use a lambda expression with QThread and QtConcurrent and to my surprise both worked, and I stuck with QThread in the end. The result looks like this:

                                  QThread* thread = QThread::create([](std::string nameString){zipUtils::unzip_v2(nameString.c_str());}, tempP.filename().string());
                                          thread->start();
                                  

                                  This won't freeze the gui anymore and performance is alright (although from time to time the system seems to struggle a bit, maybe it's something I can fix later). Anyway, thank you for your help, without this whole insight I would still be stuck :).

                                  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