search

ncssar / radiolog

SAR radio log program
Other
13 stars 3 forks source link

lag at accept for first entry of new team, increasing with team count #670

Closed caver456 closed 9 months ago

caver456 commented 11 months ago

Noticed during #391, but confirmed that the lag was there in an earlier version (3.7.0):

caver456 commented 11 months ago

Not sure if this is related to setStyleSheet, but, looking through radiolog.py for possible looping calls to setStyleSheet:

caver456 commented 11 months ago

Also see this comment in #391 - three refactor ideas regarding styleSheets / colors / appearance / statusAppearanceDict

caver456 commented 10 months ago

Noticed some likely opportunities to address this lag, while working on #612. Basically, right now rebuildTabs calls addTab for every callsign, whenever a new team is added. The call tree looks something like this:

newEntry
 calls newEntryProcessTeam
 which calls newTeam if needed
 which calls rebuildTabs
 which calls addTab for all tabs
 which (after #612 fix) calls redrawTables for the newly added table only

Seems like addTab should only be called when there's a new tab to be added. Extra calls to addTab could be a source of extra lag at new team, increasing with team count.

caver456 commented 10 months ago

rebuildTabs removes all tabs from the bar and adds them all back in the proper sequence. Changing this might be invasive and might be difficult to verify. Do some profiling first, to see if this is the right place to invest effort.

caver456 commented 10 months ago

cprofile output (up through addTab) for loading a session that has 13 callsigns in 2 groups:

(15 calls to addTab is probably due to one for the dummy and one for the separator)

   Ordered by: cumulative time

   ncalls  tottime  percall  cumtime  percall filename:lineno(function)
        2    0.002    0.001   12.212    6.106 radiolog.py:1(<module>)
    382/1    0.002    0.000   12.182   12.182 {built-in method builtins.exec}
        1    0.019    0.019   11.383   11.383 radiolog.py:9415(main)
        8    0.055    0.007    2.297    0.287 radiolog.py:3797(keyPressEvent)
        5    2.149    0.430    2.217    0.443 {built-in method exec_}
        1    0.002    0.002    2.061    2.061 radiolog.py:724(__init__)
        1    0.003    0.003    1.933    1.933 radiolog.py:5832(restore)
        1    0.016    0.016    1.836    1.836 radiolog.py:4132(load)
        1    0.002    0.002    1.249    1.249 radiolog.py:1412(checkForContinuedIncident)
        1    0.005    0.005    1.051    1.051 radiolog.py:3885(closeEvent)
       35    0.001    0.000    0.920    0.026 __init__.py:1(<module>)
   439/20    0.005    0.000    0.792    0.040 <frozen importlib._bootstrap>:1022(_find_and_load)
   437/20    0.002    0.000    0.792    0.040 <frozen importlib._bootstrap>:987(_find_and_load_unlocked)
   406/23    0.002    0.000    0.776    0.034 <frozen importlib._bootstrap>:664(_load_unlocked)
   364/20    0.002    0.000    0.766    0.038 <frozen importlib._bootstrap_external>:877(exec_module)
   492/26    0.001    0.000    0.764    0.029 <frozen importlib._bootstrap>:233(_call_with_frames_removed)
        1    0.000    0.000    0.610    0.610 radiolog.py:4773(rebuildTabs)
       15    0.003    0.000    0.610    0.041 radiolog.py:4949(addTab)

One way to interpret this is that the 15 calls to addTab took up a total of a third of the time used by the load function. This does imply that minimizing the number of calls to addTab should have a good payoff.

caver456 commented 10 months ago

Confirming that this would be a good investment of effort: adding one new team (one entry for a new callsign) increases the number of addTab calls to 31 (15 + 16) which more than doubles the time spent in addTab and rebuildTabs:

   Ordered by: cumulative time

   ncalls  tottime  percall  cumtime  percall filename:lineno(function)
        2    0.002    0.001   20.271   10.135 radiolog.py:1(<module>)
    382/1    0.002    0.000   20.242   20.242 {built-in method builtins.exec}
        1    0.017    0.017   19.417   19.417 radiolog.py:9415(main)
        6    4.714    0.786    4.853    0.809 {built-in method exec_}
        1    0.008    0.008    2.964    2.964 radiolog.py:3885(closeEvent)
        5    0.041    0.008    2.533    0.507 radiolog.py:3797(keyPressEvent)
        1    0.002    0.002    2.206    2.206 radiolog.py:724(__init__)
        1    0.006    0.006    1.889    1.889 radiolog.py:5832(restore)
        1    0.015    0.015    1.795    1.795 radiolog.py:4132(load)
        1    0.002    0.002    1.453    1.453 radiolog.py:1412(checkForContinuedIncident)
        2    0.000    0.000    1.235    0.617 radiolog.py:4773(rebuildTabs)
       31    0.005    0.000    1.218    0.039 radiolog.py:4949(addTab)
caver456 commented 10 months ago

addTab doesn't need refactoring - we just need to reduce the number of times it's called to a minimum. Currently, addTab is only called from rebuildTabs, which is called from two places: 1) the end of load, and 2) newTeam.

The main problem is that every call to newTeam calls rebuildTabs which rebuilds the entire tab bar and all tab table widgets.

Instead, newTeam should only call addTab for the tab to be added.

That change is easy, but it causes some errors because not all the tabs are rebuilt in the same sequence as rebuildTabs. Specifically, the dummy tab is never added, so the tab sequence is out of sync, which shows up as a setStyleSheet error.

So: call rebuildTabs at the end if MainWindow.init to make sure the dummy tab gets added. Except, this results in a visible 'dummy' tab. But we're on the right track...

caver456 commented 10 months ago

Getting closer. Apparently there's still something that happened in the first rebuildTabs (after the first new entry in a new session) that isn't getting done by the 'replacement code' which only calls addTab for the one new tab in question:

image

caver456 commented 10 months ago

After several different attempts, it looks like the solution might be pretty easy:

old:

        if not self.loadFlag:
            self.rebuildTabs()

new:

        if not self.loadFlag:
            # 670 - for the first team of a new session, follow the old behavior and rebuild the entire tab bar;
            #  for subsequent teams, only add the tab for the new team 
            if self.ui.tabWidget.tabBar().count()==0:
                self.rebuildTabs()
            else:
                self.addTab(extTeamName)

need to do some more testing, but, this does get rid of the lag and seems to work correctly.

caver456 commented 10 months ago

With the new code in place: unhiding of hidden tab seems to have some issues. Unhiding of the rightmost tab results in scroll arrows. This is after hideinf team 4 then unhide-via-new-fleetsync-call of team 4:

image

Unhiding of other tabs (leftmost or otherwise) seems fine. Also this isn't consistent. Here's an apparent sequence of things:

  1. hide-then-unhide rightmost: scroll arrows, and entire bar shifted/clipped left as in the pic
  2. hide-then-unhide leftmost: all good
  3. hide-then-unhide rightmost: all good
  4. hide-then-unhide rightmost: buggered as in 1 and as in the pic
  5. hide-then-unhide rightmost: buggered

Also: when buggered: activating a different tab, either by clicking the tab or by clicking in the teams sidebar or by accepting a new entry from that other team, fixes the display. This might show an easier path to fixing the problem.

Confirmed that this issue doesn't exist with the old code (always rebuildTabs on every new team). So the easy solution would be to call rebuildTabs after unhiding, but, that would introduce the lag when unhiding.

caver456 commented 9 months ago

solved the display issues by selecting tab 0 before adding the unhidden tab; if the tab count is tiny (if unhiding the only tab), call rebuildTabs since the lag will be irrelevant.