Anonymous | Login | Signup for a new account | 2024-11-03 06:15 UTC |
My View | View Issues | Change Log | Roadmap | Doomseeker Issue Support Ranking | Rules | My Account |
View Issue Details [ Jump to Notes ] | [ Issue History ] [ Print ] | ||||||||||||
ID | Project | Category | View Status | Date Submitted | Last Update | ||||||||
0004421 | Doomseeker | [All Projects] Epic | public | 2024-10-09 20:48 | 2024-11-02 22:25 | ||||||||
Reporter | Zalewa | ||||||||||||
Assigned To | Zalewa | ||||||||||||
Priority | normal | Severity | feature | Reproducibility | N/A | ||||||||
Status | assigned | Resolution | open | ||||||||||
Platform | OS | OS Version | |||||||||||
Product Version | 1.4.1 | ||||||||||||
Target Version | 1.5.0 | Fixed in Version | |||||||||||
Summary | 0004421: Doomseeker 1.5.0 release | ||||||||||||
Description | Let's release Doomseeker 1.5.0. The detailed instructions are in the checklist attachment, as usual. | ||||||||||||
Additional Information | Doomseeker release 1.4.1: 0004109 Doomseeker release 1.4.0: 0004082 | ||||||||||||
Attached Files | Ip2CRegression.png [^] (429,343 bytes) 2024-10-14 03:23 release-checklist-1.5-v2.txt [^] (2,792 bytes) 2024-10-27 15:31 [Show Content] | ||||||||||||
Notes | |
(0024060) Blzut3 (administrator) 2024-10-14 03:22 |
Fixed Qt6 build issues. It appears that the std::sort that crashed was because the return values were inverted. Am I crazy or are there regressions with the IP2C handling? On 1.4.1 most servers are identified and Doomseeker starts querying instantly. With the latest git Doomseeker hangs completely unresponsive for several seconds at start up and then most servers appear to fail country lookup? |
(0024062) Zalewa (developer) 2024-10-14 19:22 |
The V3 of the IP2C database format was in 1.4.1 already, and I don't see any changes to this code after that. Maybe there's some parser-related timing issue that I'm lucky enough not to hit? The parsing is still happening in a background thread here. Or maybe the IP2C dat file was mangled? I've just now deployed a new version of the IP2C database. See if the problem persists after you update. For me it works correctly. I moved the previous .dat file to the "ip2c/old/2023-03-27" directory on the website's server, just in case. As for the hang: I have noticed a freeze after you press the server refresh button and right before the server table is filled with Zandronum servers. There being 550 Zandronum servers doesn't help with the Qt table performance, but even with Odamex's 180 servers there is a brief but noticeable freeze. This thing could be looked into. |
(0024063) Blzut3 (administrator) 2024-10-14 22:34 edited on: 2024-10-14 22:35 |
The hang you notice may indeed be the same, I've traced the change to 8e0d117c8e40f4dccac5c9810ff969ef24dac67d, which seems curious but it does seem to be related to regular expression processing for each of the table rows. Subset of the backtrace I got from breaking while hung:0x00007ffff678ac7d in QRegularExpression::isValid() const () from /lib/x86_64-linux-gnu/libQt5Core.so.5 So that's definitely a regression to look at. Still trying to figure out what the deal is with IP2C. It might not be a regression strictly speaking as rebuilding 1.4.1 seems to be broken for me (even built in my Ubuntu 18.04 chroot), but my deb package still works fine. They're both parsing the same file. |
(0024064) Blzut3 (administrator) 2024-10-15 01:27 edited on: 2024-10-15 02:39 |
It does appear the IP2C issue is timing related. Making sure the IP2C parsing doesn't finish until after the query on startup is finished seems to make everything resolve correctly. Presumably something is going wrong if the IP2C parsing finishes mid refresh. As for 1.4.1 deb working, it seems Release build related. So if it parses fast enough things seem to be OK too. It does seem to hang issue makes it considerably more likely to get into this state, but hard to say. Edit: Looks like the cause is IP2CLoader::onParsingFinished is never called. Haven't figured out why it works in some cases but not others. |
(0024065) Zalewa (developer) 2024-10-15 16:32 edited on: 2024-10-15 16:33 |
Running a regexp in a should-be-light-weight function was probably not the best idea. I just moved the piece that derives the canonical name, if the plugin didn't specify one, to the init() method. 'https://bitbucket.org/Doomseeker/doomseeker/commits/e686748435ec4a1550b6ae0ca658b8aced89da1f [^]' This has sped things up noticeably already, but there's still something off. Adding a qDebug() with a static counter to EnginePlugin::nameCanonical() and refreshing Chocolate Doom servers has logged that the method was called roughly 2400 times. Chocolate Doom has 17 servers, and 4 of them don't respond... |
(0024066) Zalewa (developer) 2024-10-15 18:29 |
Another optimization that avoids calling gPlugins->pluginIndexFromName(): 'https://bitbucket.org/Doomseeker/doomseeker/commits/c070ba22c5300f5575bca901121f7a45691854cf [^]' Plus it sits entirely in the header, allowing the compiler to do its own optimizations. I think that there may still be some room for improvement here. Maybe the server identity could be precalculated into an int hash code, and this could be compared first before the exact details are compared? |
(0024067) Blzut3 (administrator) 2024-10-15 22:46 |
Confirming that these optimizations are enough to fix the hang for me. |
(0024069) Zalewa (developer) 2024-10-19 11:44 edited on: 2024-10-19 11:45 |
I can't reproduce the IP2C loading problem. Even if I introduce an artificial sleep to the IP2CParsingThread, it still finishes correctly and the flags do eventually change from '?' to proper country flags. I tried enabling all plugins, all server refresh on startup, and IP2C update check on startup, and I tried Doomseeker both before and after the server table performance fixes. The onParsingFinished() slot is tied to QThread::finished() signal. If it not being called is the problem here, I can see this happening only in two cases: 1. The IP2CParsingThread never finishes, or 2. the signal is disconnected prematurely. There's one explicit disconnect, but it's in a place that also starts a new parsing thread, so the new thread should emit the signal eventually. Another potential cause may be that the finished() signal is also tied to the thread's QObject::deleteLater(). I'm guessing here, but maybe this causes a disconnect and prevents the signal from reaching the other slot? If that is the case, then maybe this patch will help:
|
(0024071) Blzut3 (administrator) 2024-10-19 21:17 edited on: 2024-10-19 21:18 |
I've commented out the deleteLater connection to no avail so I think I already ruled that out. Similarly setting the onParsingFinished connection to a direct connection works (ignoring possible threading issues), so it's being emitted. But there's a certain sequence of things where it seems the queued connection gets dropped (and using a blocking connection doesn't change anything here). I'm still completely unsure what's going on but my working theory right now is that the sequence of events looks something like this: * Startup refresh triggers. * Master server responds, server queries begin. * IP2C finishes parsing. * Something in main thread happens here that causes the event queue to be flushed If that something happens fast enough to occur before IP2C parsing finishes, things work. If IP2C parsing takes long enough that it completes after, things work. |
(0024072) Blzut3 (administrator) 2024-10-20 04:43 |
Still not sure why the call is being dropped, but one sweep it under the rug solution appears to be to move "d->ip2cLoader->load();" to MainWindow::postInitAppStartup. Not knowing the root cause is really uncomfortable though. I don't think there's any reason why the main event loop would need to be run first before queued connections work, and that wouldn't explain why it works if it completes well before the event loop starts. |
(0024073) Zalewa (developer) 2024-10-20 21:07 |
One last thing I wanted to fix before the release was 0004173, but it's not as trivial as I'd hoped it to be. Unless you have some better idea than what I outlined in 0004173:0024070, Blzut, then I will postpone it for a future release. For Windows build I will distribute 1.5.0 with Qt 5.7 still, because this is the final version that runs on Windows XP, but I think that it's time to retire Windows XP support after that. I need to consider how I want to deal with the auto-updates here. |
(0024075) Blzut3 (administrator) 2024-10-21 01:47 |
I think I finally figured out the root cause of the IP2C issue. (Note line numbers in stack trace are probably nonsense given all the measurement code I added to trace this down in the first place.)0x00005555558582f2 in IP2CLoader::~IP2CLoader (this=0x555556006ed0, __in_chrg=<optimized out>) at /home/blzut3/Code/Doomseeker/src/core/ip2c/ip2cloader.cpp:114 The IP2CLoader::finished signal is emitted in multiple places, which connects to MainWindow::ip2cJobsFinished. Depending on the specific timing of queued slot execution, this can delete the IP2CLoader mid load and that causes the thread's signals to be disconnected. Also ACK on the Windows XP note. Do you plan to jump to Qt6 after? |
(0024077) Zalewa (developer) 2024-10-21 16:02 edited on: 2024-10-21 20:18 |
Quote Yes, I will update the compiler too. I should probably explore first if there will be compatibility issues with Windows 7 (which will become the new XP now). The docs say that the platform is not supported, even with Qt 6.0, but this doesn't mean that the program won't run ;) 'https://doc.qt.io/archives/qt-6.0/supported-platforms.html#windows [^]' |
(0024078) Blzut3 (administrator) 2024-10-22 00:02 |
Well if you end up deciding to stick with Qt 5.15 for Windows 7 that's fine too. Guess it's worth writing some notes on the status for Linux: KDE 6 is just starting to roll out so it missed the LTS cycle. Debian 13 and Ubuntu 26.04 should have a solid Qt6 experience, so ideally I'd like Qt 5.15 supported until then. Debian 11 with backports repo, Debian 12, and Ubuntu 22.04+ all ship Qt 6.2+. Theming might be a bit raw on them though, which I think is fine if it's not the latest OS release. Should probably also mention that Ubuntu 20.04 is Qt 5.12, so until May 2025 that's my ideal minimum right now. But at our current cadence that might be irrelevant by the next release. |
(0024079) Zalewa (developer) 2024-10-22 18:32 |
I created ticket 0004431 for the IP2C issue, quoting your relevant comments as the description.Quote from Blzut3 Once I'm ready to migrate I shall set the Docker images used in Bitbucket's Pipelines to this version then. This is very useful as a build compatibility check harness. |
(0024106) Zalewa (developer) 2024-10-27 16:32 |
I did all the steps from "1", except for those where the files are deployed to Doomseeker's website because my write mount is temporarily unavailable. Time for step "2". Also, I replaced the checklist file with a v2 because the previous checklist still referred to SRB2 plugin, which was dealt with in the previous release. I just uploaded it without reading it carefully. |
(0024114) Blzut3 (administrator) 2024-11-02 22:25 |
macOS, Linux, and source packages are uploaded. |
Only registered users can voice their support. Click here to register, or here to log in. | |
Supporters: | No one explicitly supports this issue yet. |
Opponents: | No one explicitly opposes this issue yet. |
Issue History | |||
Date Modified | Username | Field | Change |
2024-10-09 20:48 | Zalewa | New Issue | |
2024-10-09 20:48 | Zalewa | File Added: release-checklist-1.5.txt | |
2024-10-09 20:49 | Zalewa | Severity | minor => feature |
2024-10-14 03:22 | Blzut3 | Note Added: 0024060 | |
2024-10-14 03:23 | Blzut3 | File Added: Ip2CRegression.png | |
2024-10-14 19:22 | Zalewa | Note Added: 0024062 | |
2024-10-14 22:34 | Blzut3 | Note Added: 0024063 | |
2024-10-14 22:35 | Blzut3 | Note Edited: 0024063 | View Revisions |
2024-10-15 01:27 | Blzut3 | Note Added: 0024064 | |
2024-10-15 01:30 | Blzut3 | Note Edited: 0024064 | View Revisions |
2024-10-15 02:39 | Blzut3 | Note Edited: 0024064 | View Revisions |
2024-10-15 16:32 | Zalewa | Note Added: 0024065 | |
2024-10-15 16:33 | Zalewa | Note Edited: 0024065 | View Revisions |
2024-10-15 18:29 | Zalewa | Note Added: 0024066 | |
2024-10-15 22:46 | Blzut3 | Note Added: 0024067 | |
2024-10-19 11:44 | Zalewa | Note Added: 0024069 | |
2024-10-19 11:45 | Zalewa | Note Edited: 0024069 | View Revisions |
2024-10-19 21:17 | Blzut3 | Note Added: 0024071 | |
2024-10-19 21:18 | Blzut3 | Note Edited: 0024071 | View Revisions |
2024-10-20 04:43 | Blzut3 | Note Added: 0024072 | |
2024-10-20 21:07 | Zalewa | Note Added: 0024073 | |
2024-10-21 01:47 | Blzut3 | Note Added: 0024075 | |
2024-10-21 16:02 | Zalewa | Note Added: 0024077 | |
2024-10-21 20:18 | Zalewa | Note Edited: 0024077 | View Revisions |
2024-10-22 00:02 | Blzut3 | Note Added: 0024078 | |
2024-10-22 18:32 | Zalewa | Note Added: 0024079 | |
2024-10-23 17:26 | Zalewa | Assigned To | => Zalewa |
2024-10-23 17:26 | Zalewa | Status | new => assigned |
2024-10-27 15:31 | Zalewa | File Added: release-checklist-1.5-v2.txt | |
2024-10-27 15:31 | Zalewa | File Deleted: release-checklist-1.5.txt | |
2024-10-27 16:30 | Zalewa | Assigned To | Zalewa => Blzut3 |
2024-10-27 16:32 | Zalewa | Note Added: 0024106 | |
2024-11-02 22:25 | Blzut3 | Assigned To | Blzut3 => Zalewa |
2024-11-02 22:25 | Blzut3 | Note Added: 0024114 |
Copyright © 2000 - 2024 MantisBT Team |