-
-
Notifications
You must be signed in to change notification settings - Fork 60
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Qiqqa performance: hold off background tasks until the application has completely rendered the UI #55
Comments
Progress... of sorts 🤔I'm running into additional nastiness with this. The thread-safety edits I did to date are under suspicion as it smells like a deadlock somewhere but after instrumenting all However, the same 'dreadful long UI wait' happens when I move that huge library aside and let Qiqqa only load one 'evil library' @ 1 document only plus an old intranet lib @ 37 articles: this should be a snap but isn't with my codebase today. Performance-wise there are a few things that start to come out of the many hours of intrumenting, debugging and performance analysis:
Well, that's progress for ya. Back to going through the code; this ends this rant, which at least serves as a checklist for me as things get pretty involved now. [EDIT]Turns out most of the trouble was caused by UI cruft instead of locks pending forever: s commit SHA-1: 2e9c486 and SHA-1: ea2f8e7. This goes to show that initial assumptions, though sticky for a while, are no guarantee you'll find reality to match. 🤡 At the time of writing this new state of affairs must still be tested with the large 29K+ library... |
… while I dig further to uncover the *slowtard* culprits. Mother Of All Commits. Can be split up into multiple commits if needed later on in a separate branch. Actions done in the past days while hunting the shite: - `library.GetDocumentByFingerprint(fingerprint)` can get you a **NULL** and quite a few spots in the code didn't account for this; ran into several crashes along the way due to this. Now code has been augmented with Error logging, etc. to at least report these occurrences as they often hint at internal sanity issues of the library or codebase. - using C# 6.0 language constructs where it helps readability. I'm happy the `?` and `??` operators are around today. - **IMPORTANT**: instrument the Hell out of those C# `lock(...)` constructs to detect if any of those buggers is deadlocking or at least retarding on me. The code idiom looks like this: Utilities.LockPerfTimer l1_clk = Utilities.LockPerfChecker.Start(); lock (thread_lock) { l1_clk.LockPerfTimerStop(); where the `LockPerfChecker.Start()` call sets up an async timer to fire when waiting for the `.LockPerfTimerStop()` takes too much time. Any such occurrence is then logged to Qiqqa.log - *NUKED* the Chat timer ticker as it hampered the debugging sessions no end and is pretty useless right now anyway. TODO: Uncomment that `#if false` in ChatControl when I'm done. - fixed a NULL string crash in the Window Size Restore logic (StandardWindow.cs) - added more Debug level logging to monitor the action while the large Library fills up - `BlackWhiteListManager`: improved error reporting: either the file isn't there *or* it's corrupt and we should be able to identify both situations from the log. - `FolderWatcher`: rename the ill-named `previous_folder_to_watch` variable. - `FolderWatcher`: `file_system_watcher.Path = folder_to_watch;` will crash when you happen to have configured a non-existing path to watch. The new code takes proper care of this situation. - `NotificationManager.Notification`: when the tooltip is NULL, it will use the notification text itself instead. Corrected the invocations of this API to use this feature and reduce the amount of string duplication in the source code. - use the preferred/MS-advised idiom for `lock(xxx_lock) { ...use xxx...}` where a separate `object` is created for the lock thoughout the codebase: this prevents odd failures with locking Dictionary-types etc. -- I haven't explicitly scanned the codebase to uncover all but most such situations have been corrected. - `Library:BuildFromDocumentRepository()`: log the progress loading the documents once every X seconds rather than every 250 documents: sometimes loading is fast, sometimes slow and I'd rather observe such in the logfile. - `Library` performance: `internal HashSet<string> GetAllDocumentFingerprints()` has been recoded and should be (marginally) faster now. - `Library` `PDFDocuments` performance: several controls used code like `library.PDFDocuments.Count` which is rather costly as it copies an entire set just to measure its size. Using the `library.PDFDocuments_IncludingDeleted_Count` API instead. - `Library`: `internal HashSet<string> GetDocumentFingerprintsWithKeyword(string keyword)` has been augmented with a `Publication` match check and has been made thread-safe like the rest of them: all code accessing the `pdf_documents` member should be thread-safe or you're in for spurious trouble. - `LibraryFilterControl`: encoded all sorting mode delegates the same way: as functions in `PDFDocumentListSorters`. (Code consistency) - As I ran into several crashes in the application init phase, a few *singleton objects*' idiom has changed from class WebLibraryManager { public static WebLibraryManager Instance = new WebLibraryManager(); to class WebLibraryManager { private static WebLibraryManager __instance = null; public static WebLibraryManager Instance { get { if (null == __instance) { __instance = new WebLibraryManager(); } return __instance; } } which results in a singleton with 'init on first demand' behaviour, hence singleton instances which initialize later during the application init phase, where various internal APIs become safely available. (See also the `Logging` entry below) - `PDFDocumentCitationManager`: despite a slew of `lock(...)` code, there was still thread-UNSAFE usage in there in the `CloneFrom()` method, due to the lock being **instance**-particular. This has been resolved in the class' code. - `GoogleBibTexSnifferControl.xaml.cs`: fixed another spurious NULL-induced crash - `FeatureTracking:GoogleAnalysicsSubmitter` begets a 403 (Forbidden) from Google Analytics. Report such in the logfile instead of letting it pass silently. (Not that I care that much about feature tracking right now, but this was part of the larger code review re HTTP access from Qiqqa) - `StatusBar`: move the thread-creating/queueing `Dispatcher.BeginInvoke(...)` call outside the critical section: this will prevent potential deadlock scenarios (not sure if this is one of the retarders, but at least it popped up during code review). - BibTeXAssembler/Parser: append some harmless whitespace at the end to help reduce the number of out-of-bounds exceptions in the parser/lexer while keeping the code simple - `GUITools::public static void ScrollToTop(ListView listview)` was seriously haranguing the logfile with a slew of deep-level exceptions while large libraries are loading and the UI is set up in Qiqqa, due to the listview not yet being ready to receive such attention. After trying a few things, I *think* I got it fixed by checking the `Height` property for `NaN` to preclude the entire offending code chunk. **Interestingly enough, said code chunk will now *never execute* due to the premature UI updating done via those `.Library = library` code lines mentioned in jimmejardine#55 (comment) See the comment chunk above this edit for more info. - `Logging`: ohhhh boy! It started with working on the crashes detected at early app init time, where there were several `Logging.Info()` calls which would trigger a crash in the `Logger` constructor code, due to other bits of the code not being ready yet. Now the `Logging` class buffers any such 'premature' logging reports in a RAM buffer and dumps them to logfile as soon as it comes available. Meanwhile... - `Logging`: **WEIRDING-OUT ALERT** ... seems to be a **very odd class** as any methods added to it seem to **automagickally lock** i.e. are thread-safety-protected, but I cannot find out how this was done -- it's been too long since I last went neck-deep into .NET and I seem to recall some behaviour like that from anything log4net-related from back in my days with .NET and log4net. Anyhow, after gnashing my teeth and killing a few debuggers I went for the alternative: the work-around. Those functions have been moved into the new `LockPerfChecker.cs` file/class, which is nice re code organization as well. Nevertheless I believe I should those **weird** lockups I had with lock test code in `LogError` there while I was debugging and testing the new `lock()` instrumentation code. Very odd indeed, unless one assumes the entire `Logging` class is thread-lock-protected in *bulk*. **WEIRDING-OUT ALERT** - `QueueUserWorkItem`: disable test code in there which is only slowing us down. - removed unused source files, e.g. <Compile Include="Finance\YieldSolverAnnualised.cs" /> <Compile Include="Finance\YieldSolverContinuous.cs" /> <Compile Include="Finance\YieldSolver.cs" />
…ScrollToTop. Dang! jimmejardine#55 (comment) & commit SHA-1: 656bd92
…sing the keyboard: it's the repeated invocation of the code line `ThemeTabItem.AddToApplicationResources(app)` for each individual control being instantiated that killed the focus semi-randomly. That code was introduced/activated that way a while ago to better support the MSVS2019 Designer -- which should still work as before. - fixes several issues reported in jimmejardine#55 (comment) : the keyboard scrolling no longer suffers from long delays -- where I first suspected thread locking it turns out to be a hassle with WPF (grrrrrr); I cannot explain **how exactly** WPF caused the lethargic slowdown, but the fact is that it's gone now. The fact I cannot explain this makes this codebase brittle from my point of view. Alas. - it *looks* like I can gt away with a bit of a performance boost as that `listview.UpdateLayout();` from `GUITools::ScrollToTop()` looks like it can be safely moved into the IsVisibleChanged handler for said ListView(s)... hmmmmm
… while I dig further to uncover the *slowtard* culprits. Mother Of All Commits. Can be split up into multiple commits if needed later on in a separate branch. Actions done in the past days while hunting the shite: - `library.GetDocumentByFingerprint(fingerprint)` can get you a **NULL** and quite a few spots in the code didn't account for this; ran into several crashes along the way due to this. Now code has been augmented with Error logging, etc. to at least report these occurrences as they often hint at internal sanity issues of the library or codebase. - using C# 6.0 language constructs where it helps readability. I'm happy the `?` and `??` operators are around today. - **IMPORTANT**: instrument the Hell out of those C# `lock(...)` constructs to detect if any of those buggers is deadlocking or at least retarding on me. The code idiom looks like this: Utilities.LockPerfTimer l1_clk = Utilities.LockPerfChecker.Start(); lock (thread_lock) { l1_clk.LockPerfTimerStop(); where the `LockPerfChecker.Start()` call sets up an async timer to fire when waiting for the `.LockPerfTimerStop()` takes too much time. Any such occurrence is then logged to Qiqqa.log - *NUKED* the Chat timer ticker as it hampered the debugging sessions no end and is pretty useless right now anyway. TODO: Uncomment that `#if false` in ChatControl when I'm done. - fixed a NULL string crash in the Window Size Restore logic (StandardWindow.cs) - added more Debug level logging to monitor the action while the large Library fills up - `BlackWhiteListManager`: improved error reporting: either the file isn't there *or* it's corrupt and we should be able to identify both situations from the log. - `FolderWatcher`: rename the ill-named `previous_folder_to_watch` variable. - `FolderWatcher`: `file_system_watcher.Path = folder_to_watch;` will crash when you happen to have configured a non-existing path to watch. The new code takes proper care of this situation. - `NotificationManager.Notification`: when the tooltip is NULL, it will use the notification text itself instead. Corrected the invocations of this API to use this feature and reduce the amount of string duplication in the source code. - use the preferred/MS-advised idiom for `lock(xxx_lock) { ...use xxx...}` where a separate `object` is created for the lock thoughout the codebase: this prevents odd failures with locking Dictionary-types etc. -- I haven't explicitly scanned the codebase to uncover all but most such situations have been corrected. - `Library:BuildFromDocumentRepository()`: log the progress loading the documents once every X seconds rather than every 250 documents: sometimes loading is fast, sometimes slow and I'd rather observe such in the logfile. - `Library` performance: `internal HashSet<string> GetAllDocumentFingerprints()` has been recoded and should be (marginally) faster now. - `Library` `PDFDocuments` performance: several controls used code like `library.PDFDocuments.Count` which is rather costly as it copies an entire set just to measure its size. Using the `library.PDFDocuments_IncludingDeleted_Count` API instead. - `Library`: `internal HashSet<string> GetDocumentFingerprintsWithKeyword(string keyword)` has been augmented with a `Publication` match check and has been made thread-safe like the rest of them: all code accessing the `pdf_documents` member should be thread-safe or you're in for spurious trouble. - `LibraryFilterControl`: encoded all sorting mode delegates the same way: as functions in `PDFDocumentListSorters`. (Code consistency) - As I ran into several crashes in the application init phase, a few *singleton objects*' idiom has changed from class WebLibraryManager { public static WebLibraryManager Instance = new WebLibraryManager(); to class WebLibraryManager { private static WebLibraryManager __instance = null; public static WebLibraryManager Instance { get { if (null == __instance) { __instance = new WebLibraryManager(); } return __instance; } } which results in a singleton with 'init on first demand' behaviour, hence singleton instances which initialize later during the application init phase, where various internal APIs become safely available. (See also the `Logging` entry below) - `PDFDocumentCitationManager`: despite a slew of `lock(...)` code, there was still thread-UNSAFE usage in there in the `CloneFrom()` method, due to the lock being **instance**-particular. This has been resolved in the class' code. - `GoogleBibTexSnifferControl.xaml.cs`: fixed another spurious NULL-induced crash - `FeatureTracking:GoogleAnalysicsSubmitter` begets a 403 (Forbidden) from Google Analytics. Report such in the logfile instead of letting it pass silently. (Not that I care that much about feature tracking right now, but this was part of the larger code review re HTTP access from Qiqqa) - `StatusBar`: move the thread-creating/queueing `Dispatcher.BeginInvoke(...)` call outside the critical section: this will prevent potential deadlock scenarios (not sure if this is one of the retarders, but at least it popped up during code review). - BibTeXAssembler/Parser: append some harmless whitespace at the end to help reduce the number of out-of-bounds exceptions in the parser/lexer while keeping the code simple - `GUITools::public static void ScrollToTop(ListView listview)` was seriously haranguing the logfile with a slew of deep-level exceptions while large libraries are loading and the UI is set up in Qiqqa, due to the listview not yet being ready to receive such attention. After trying a few things, I *think* I got it fixed by checking the `Height` property for `NaN` to preclude the entire offending code chunk. **Interestingly enough, said code chunk will now *never execute* due to the premature UI updating done via those `.Library = library` code lines mentioned in jimmejardine#55 (comment) See the comment chunk above this edit for more info. - `Logging`: ohhhh boy! It started with working on the crashes detected at early app init time, where there were several `Logging.Info()` calls which would trigger a crash in the `Logger` constructor code, due to other bits of the code not being ready yet. Now the `Logging` class buffers any such 'premature' logging reports in a RAM buffer and dumps them to logfile as soon as it comes available. Meanwhile... - `Logging`: **WEIRDING-OUT ALERT** ... seems to be a **very odd class** as any methods added to it seem to **automagickally lock** i.e. are thread-safety-protected, but I cannot find out how this was done -- it's been too long since I last went neck-deep into .NET and I seem to recall some behaviour like that from anything log4net-related from back in my days with .NET and log4net. Anyhow, after gnashing my teeth and killing a few debuggers I went for the alternative: the work-around. Those functions have been moved into the new `LockPerfChecker.cs` file/class, which is nice re code organization as well. Nevertheless I believe I should those **weird** lockups I had with lock test code in `LogError` there while I was debugging and testing the new `lock()` instrumentation code. Very odd indeed, unless one assumes the entire `Logging` class is thread-lock-protected in *bulk*. **WEIRDING-OUT ALERT** - `QueueUserWorkItem`: disable test code in there which is only slowing us down. - removed unused source files, e.g. <Compile Include="Finance\YieldSolverAnnualised.cs" /> <Compile Include="Finance\YieldSolverContinuous.cs" /> <Compile Include="Finance\YieldSolver.cs" />
…ScrollToTop. Dang! jimmejardine#55 (comment) & commit SHA-1: 656bd92
…sing the keyboard: it's the repeated invocation of the code line `ThemeTabItem.AddToApplicationResources(app)` for each individual control being instantiated that killed the focus semi-randomly. That code was introduced/activated that way a while ago to better support the MSVS2019 Designer -- which should still work as before. - fixes several issues reported in jimmejardine#55 (comment) : the keyboard scrolling no longer suffers from long delays -- where I first suspected thread locking it turns out to be a hassle with WPF (grrrrrr); I cannot explain **how exactly** WPF caused the lethargic slowdown, but the fact is that it's gone now. The fact I cannot explain this makes this codebase brittle from my point of view. Alas. - it *looks* like I can gt away with a bit of a performance boost as that `listview.UpdateLayout();` from `GUITools::ScrollToTop()` looks like it can be safely moved into the IsVisibleChanged handler for said ListView(s)... hmmmmm
… while I dig further to uncover the *slowtard* culprits. Mother Of All Commits. Can be split up into multiple commits if needed later on in a separate branch. Actions done in the past days while hunting the shite: - `library.GetDocumentByFingerprint(fingerprint)` can get you a **NULL** and quite a few spots in the code didn't account for this; ran into several crashes along the way due to this. Now code has been augmented with Error logging, etc. to at least report these occurrences as they often hint at internal sanity issues of the library or codebase. - using C# 6.0 language constructs where it helps readability. I'm happy the `?` and `??` operators are around today. - **IMPORTANT**: instrument the Hell out of those C# `lock(...)` constructs to detect if any of those buggers is deadlocking or at least retarding on me. The code idiom looks like this: Utilities.LockPerfTimer l1_clk = Utilities.LockPerfChecker.Start(); lock (thread_lock) { l1_clk.LockPerfTimerStop(); where the `LockPerfChecker.Start()` call sets up an async timer to fire when waiting for the `.LockPerfTimerStop()` takes too much time. Any such occurrence is then logged to Qiqqa.log - *NUKED* the Chat timer ticker as it hampered the debugging sessions no end and is pretty useless right now anyway. TODO: Uncomment that `#if false` in ChatControl when I'm done. - fixed a NULL string crash in the Window Size Restore logic (StandardWindow.cs) - added more Debug level logging to monitor the action while the large Library fills up - `BlackWhiteListManager`: improved error reporting: either the file isn't there *or* it's corrupt and we should be able to identify both situations from the log. - `FolderWatcher`: rename the ill-named `previous_folder_to_watch` variable. - `FolderWatcher`: `file_system_watcher.Path = folder_to_watch;` will crash when you happen to have configured a non-existing path to watch. The new code takes proper care of this situation. - `NotificationManager.Notification`: when the tooltip is NULL, it will use the notification text itself instead. Corrected the invocations of this API to use this feature and reduce the amount of string duplication in the source code. - use the preferred/MS-advised idiom for `lock(xxx_lock) { ...use xxx...}` where a separate `object` is created for the lock thoughout the codebase: this prevents odd failures with locking Dictionary-types etc. -- I haven't explicitly scanned the codebase to uncover all but most such situations have been corrected. - `Library:BuildFromDocumentRepository()`: log the progress loading the documents once every X seconds rather than every 250 documents: sometimes loading is fast, sometimes slow and I'd rather observe such in the logfile. - `Library` performance: `internal HashSet<string> GetAllDocumentFingerprints()` has been recoded and should be (marginally) faster now. - `Library` `PDFDocuments` performance: several controls used code like `library.PDFDocuments.Count` which is rather costly as it copies an entire set just to measure its size. Using the `library.PDFDocuments_IncludingDeleted_Count` API instead. - `Library`: `internal HashSet<string> GetDocumentFingerprintsWithKeyword(string keyword)` has been augmented with a `Publication` match check and has been made thread-safe like the rest of them: all code accessing the `pdf_documents` member should be thread-safe or you're in for spurious trouble. - `LibraryFilterControl`: encoded all sorting mode delegates the same way: as functions in `PDFDocumentListSorters`. (Code consistency) - As I ran into several crashes in the application init phase, a few *singleton objects*' idiom has changed from class WebLibraryManager { public static WebLibraryManager Instance = new WebLibraryManager(); to class WebLibraryManager { private static WebLibraryManager __instance = null; public static WebLibraryManager Instance { get { if (null == __instance) { __instance = new WebLibraryManager(); } return __instance; } } which results in a singleton with 'init on first demand' behaviour, hence singleton instances which initialize later during the application init phase, where various internal APIs become safely available. (See also the `Logging` entry below) - `PDFDocumentCitationManager`: despite a slew of `lock(...)` code, there was still thread-UNSAFE usage in there in the `CloneFrom()` method, due to the lock being **instance**-particular. This has been resolved in the class' code. - `GoogleBibTexSnifferControl.xaml.cs`: fixed another spurious NULL-induced crash - `FeatureTracking:GoogleAnalysicsSubmitter` begets a 403 (Forbidden) from Google Analytics. Report such in the logfile instead of letting it pass silently. (Not that I care that much about feature tracking right now, but this was part of the larger code review re HTTP access from Qiqqa) - `StatusBar`: move the thread-creating/queueing `Dispatcher.BeginInvoke(...)` call outside the critical section: this will prevent potential deadlock scenarios (not sure if this is one of the retarders, but at least it popped up during code review). - BibTeXAssembler/Parser: append some harmless whitespace at the end to help reduce the number of out-of-bounds exceptions in the parser/lexer while keeping the code simple - `GUITools::public static void ScrollToTop(ListView listview)` was seriously haranguing the logfile with a slew of deep-level exceptions while large libraries are loading and the UI is set up in Qiqqa, due to the listview not yet being ready to receive such attention. After trying a few things, I *think* I got it fixed by checking the `Height` property for `NaN` to preclude the entire offending code chunk. **Interestingly enough, said code chunk will now *never execute* due to the premature UI updating done via those `.Library = library` code lines mentioned in jimmejardine#55 (comment) See the comment chunk above this edit for more info. - `Logging`: ohhhh boy! It started with working on the crashes detected at early app init time, where there were several `Logging.Info()` calls which would trigger a crash in the `Logger` constructor code, due to other bits of the code not being ready yet. Now the `Logging` class buffers any such 'premature' logging reports in a RAM buffer and dumps them to logfile as soon as it comes available. Meanwhile... - `Logging`: **WEIRDING-OUT ALERT** ... seems to be a **very odd class** as any methods added to it seem to **automagickally lock** i.e. are thread-safety-protected, but I cannot find out how this was done -- it's been too long since I last went neck-deep into .NET and I seem to recall some behaviour like that from anything log4net-related from back in my days with .NET and log4net. Anyhow, after gnashing my teeth and killing a few debuggers I went for the alternative: the work-around. Those functions have been moved into the new `LockPerfChecker.cs` file/class, which is nice re code organization as well. Nevertheless I believe I should those **weird** lockups I had with lock test code in `LogError` there while I was debugging and testing the new `lock()` instrumentation code. Very odd indeed, unless one assumes the entire `Logging` class is thread-lock-protected in *bulk*. **WEIRDING-OUT ALERT** - `QueueUserWorkItem`: disable test code in there which is only slowing us down. - removed unused source files, e.g. <Compile Include="Finance\YieldSolverAnnualised.cs" /> <Compile Include="Finance\YieldSolverContinuous.cs" /> <Compile Include="Finance\YieldSolver.cs" />
…ScrollToTop. Dang! jimmejardine#55 (comment) & commit SHA-1: babb1bc
…sing the keyboard: it's the repeated invocation of the code line `ThemeTabItem.AddToApplicationResources(app)` for each individual control being instantiated that killed the focus semi-randomly. That code was introduced/activated that way a while ago to better support the MSVS2019 Designer -- which should still work as before. - fixes several issues reported in jimmejardine#55 (comment) : the keyboard scrolling no longer suffers from long delays -- where I first suspected thread locking it turns out to be a hassle with WPF (grrrrrr); I cannot explain **how exactly** WPF caused the lethargic slowdown, but the fact is that it's gone now. The fact I cannot explain this makes this codebase brittle from my point of view. Alas. - it *looks* like I can gt away with a bit of a performance boost as that `listview.UpdateLayout();` from `GUITools::ScrollToTop()` looks like it can be safely moved into the IsVisibleChanged handler for said ListView(s)... hmmmmm
|
…ig further to uncover the *slowtard* culprits. Mother Of All Commits. Can be split up into multiple commits if needed later on in a separate branch. Actions done in the past days while hunting the shite: - `library.GetDocumentByFingerprint(fingerprint)` can get you a **NULL** and quite a few spots in the code didn't account for this; ran into several crashes along the way due to this. Now code has been augmented with Error logging, etc. to at least report these occurrences as they often hint at internal sanity issues of the library or codebase. - using C# 6.0 language constructs where it helps readability. I'm happy the `?` and `??` operators are around today. - **IMPORTANT**: instrument the Hell out of those C# `lock(...)` constructs to detect if any of those buggers is deadlocking or at least retarding on me. The code idiom looks like this: Utilities.LockPerfTimer l1_clk = Utilities.LockPerfChecker.Start(); lock (thread_lock) { l1_clk.LockPerfTimerStop(); where the `LockPerfChecker.Start()` call sets up an async timer to fire when waiting for the `.LockPerfTimerStop()` takes too much time. Any such occurrence is then logged to Qiqqa.log - *NUKED* the Chat timer ticker as it hampered the debugging sessions no end and is pretty useless right now anyway. TODO: Uncomment that `#if false` in ChatControl when I'm done. - fixed a NULL string crash in the Window Size Restore logic (StandardWindow.cs) - added more Debug level logging to monitor the action while the large Library fills up - `BlackWhiteListManager`: improved error reporting: either the file isn't there *or* it's corrupt and we should be able to identify both situations from the log. - `FolderWatcher`: rename the ill-named `previous_folder_to_watch` variable. - `FolderWatcher`: `file_system_watcher.Path = folder_to_watch;` will crash when you happen to have configured a non-existing path to watch. The new code takes proper care of this situation. - `NotificationManager.Notification`: when the tooltip is NULL, it will use the notification text itself instead. Corrected the invocations of this API to use this feature and reduce the amount of string duplication in the source code. - use the preferred/MS-advised idiom for `lock(xxx_lock) { ...use xxx...}` where a separate `object` is created for the lock thoughout the codebase: this prevents odd failures with locking Dictionary-types etc. -- I haven't explicitly scanned the codebase to uncover all but most such situations have been corrected. - `Library:BuildFromDocumentRepository()`: log the progress loading the documents once every X seconds rather than every 250 documents: sometimes loading is fast, sometimes slow and I'd rather observe such in the logfile. - `Library` performance: `internal HashSet<string> GetAllDocumentFingerprints()` has been recoded and should be (marginally) faster now. - `Library` `PDFDocuments` performance: several controls used code like `library.PDFDocuments.Count` which is rather costly as it copies an entire set just to measure its size. Using the `library.PDFDocuments_IncludingDeleted_Count` API instead. - `Library`: `internal HashSet<string> GetDocumentFingerprintsWithKeyword(string keyword)` has been augmented with a `Publication` match check and has been made thread-safe like the rest of them: all code accessing the `pdf_documents` member should be thread-safe or you're in for spurious trouble. - `LibraryFilterControl`: encoded all sorting mode delegates the same way: as functions in `PDFDocumentListSorters`. (Code consistency) - As I ran into several crashes in the application init phase, a few *singleton objects*' idiom has changed from class WebLibraryManager { public static WebLibraryManager Instance = new WebLibraryManager(); to class WebLibraryManager { private static WebLibraryManager __instance = null; public static WebLibraryManager Instance { get { if (null == __instance) { __instance = new WebLibraryManager(); } return __instance; } } which results in a singleton with 'init on first demand' behaviour, hence singleton instances which initialize later during the application init phase, where various internal APIs become safely available. (See also the `Logging` entry below) - `PDFDocumentCitationManager`: despite a slew of `lock(...)` code, there was still thread-UNSAFE usage in there in the `CloneFrom()` method, due to the lock being **instance**-particular. This has been resolved in the class' code. - `GoogleBibTexSnifferControl.xaml.cs`: fixed another spurious NULL-induced crash - `FeatureTracking:GoogleAnalysicsSubmitter` begets a 403 (Forbidden) from Google Analytics. Report such in the logfile instead of letting it pass silently. (Not that I care that much about feature tracking right now, but this was part of the larger code review re HTTP access from Qiqqa) - `StatusBar`: move the thread-creating/queueing `Dispatcher.BeginInvoke(...)` call outside the critical section: this will prevent potential deadlock scenarios (not sure if this is one of the retarders, but at least it popped up during code review). - BibTeXAssembler/Parser: append some harmless whitespace at the end to help reduce the number of out-of-bounds exceptions in the parser/lexer while keeping the code simple - `GUITools::public static void ScrollToTop(ListView listview)` was seriously haranguing the logfile with a slew of deep-level exceptions while large libraries are loading and the UI is set up in Qiqqa, due to the listview not yet being ready to receive such attention. After trying a few things, I *think* I got it fixed by checking the `Height` property for `NaN` to preclude the entire offending code chunk. **Interestingly enough, said code chunk will now *never execute* due to the premature UI updating done via those `.Library = library` code lines mentioned in #55 (comment) See the comment chunk above this edit for more info. - `Logging`: ohhhh boy! It started with working on the crashes detected at early app init time, where there were several `Logging.Info()` calls which would trigger a crash in the `Logger` constructor code, due to other bits of the code not being ready yet. Now the `Logging` class buffers any such 'premature' logging reports in a RAM buffer and dumps them to logfile as soon as it comes available. Meanwhile... - `Logging`: **WEIRDING-OUT ALERT** ... seems to be a **very odd class** as any methods added to it seem to **automagickally lock** i.e. are thread-safety-protected, but I cannot find out how this was done -- it's been too long since I last went neck-deep into .NET and I seem to recall some behaviour like that from anything log4net-related from back in my days with .NET and log4net. Anyhow, after gnashing my teeth and killing a few debuggers I went for the alternative: the work-around. Those functions have been moved into the new `LockPerfChecker.cs` file/class, which is nice re code organization as well. Nevertheless I believe I should those **weird** lockups I had with lock test code in `LogError` there while I was debugging and testing the new `lock()` instrumentation code. Very odd indeed, unless one assumes the entire `Logging` class is thread-lock-protected in *bulk*. **WEIRDING-OUT ALERT** - `QueueUserWorkItem`: disable test code in there which is only slowing us down. - removed unused source files, e.g. <Compile Include="Finance\YieldSolverAnnualised.cs" /> <Compile Include="Finance\YieldSolverContinuous.cs" /> <Compile Include="Finance\YieldSolver.cs" />
…ScrollToTop. Dang! #55 (comment) & commit SHA-1: babb1bc
…sing the keyboard: it's the repeated invocation of the code line `ThemeTabItem.AddToApplicationResources(app)` for each individual control being instantiated that killed the focus semi-randomly. That code was introduced/activated that way a while ago to better support the MSVS2019 Designer -- which should still work as before. - fixes several issues reported in #55 (comment) : the keyboard scrolling no longer suffers from long delays -- where I first suspected thread locking it turns out to be a hassle with WPF (grrrrrr); I cannot explain **how exactly** WPF caused the lethargic slowdown, but the fact is that it's gone now. The fact I cannot explain this makes this codebase brittle from my point of view. Alas. - it *looks* like I can gt away with a bit of a performance boost as that `listview.UpdateLayout();` from `GUITools::ScrollToTop()` looks like it can be safely moved into the IsVisibleChanged handler for said ListView(s)... hmmmmm
Running background tasks after a fixed delay is okay, but with large libs on slow boxes (or when running Qiqqa in a development environment while running dev tasks, e.g. debugging or performance measuring) this initial delay is too short.
The text was updated successfully, but these errors were encountered: