KDAB / hotspot

The Linux perf GUI for performance analysis.
4.16k stars 257 forks source link

`canVisualizeJumps()` should cache the result #545

Closed GitMensch closed 1 year ago

GitMensch commented 1 year ago

In common use-cases the objdump binary won't change during execution, so it doesn't make any sense to parse its help before each disassembly.

https://github.com/KDAB/hotspot/blob/cdc66a49c80020905fd3ecdd1236e2dcf5014fb3/src/models/disassemblyoutput.cpp#L23-L34

Note: I've tried to cache the object reference (so if this doesn't changed return the last result) but with my current QT/C++ knowledge neither that nor the more expensive cache and check of the full string did work, so "open for PR and/or explanation".

milianw commented 1 year ago

why should we cache it? it's super cheap to query and parse the result after all:

 Performance counter stats for 'objdump -H':

              5.08 msec task-clock:u                     #    0.817 CPUs utilized             
                 0      context-switches:u               #    0.000 /sec                      
                 0      cpu-migrations:u                 #    0.000 /sec                      
               588      page-faults:u                    #  115.839 K/sec                     
         6,227,390      cycles:u                         #    1.227 GHz                         (42.54%)
           245,932      stalled-cycles-frontend:u        #    3.95% frontend cycles idle      
         2,467,371      stalled-cycles-backend:u         #   39.62% backend cycles idle       
        14,627,394      instructions:u                   #    2.35  insn per cycle            
                                                  #    0.17  stalled cycles per insn   
         3,055,126      branches:u                       #  601.874 M/sec                     
           103,250      branch-misses:u                  #    3.38% of all branches             (57.46%)

       0.006213254 seconds time elapsed

       0.000000000 seconds user
       0.006265000 seconds sys
GitMensch commented 1 year ago

One syscall less is a good syscall 😁, and creating a sub process is. Likely the most expensive one.

But feel free to close as unplanned.