[Bug] infinite loading loop

[kaamui]

Hi @letsfindaway,

What is the behavior with current dev for you when importing the following (extreme) document ?

https://www.ipcc.ch/report/ar6/wg3/downloads/report/IPCC_AR6_WGIII_FullReport.pdf

I'm personnally experiencing an infinite loading loop. It feels like the thumbnail generation and refreshing is a total mess at this point. Maybe some links with the cache system I'm not sure

I also noticed that the metadata is rewritten each time one of the 2042 pages is created...

Can you confirm (or not) the bad perfs/strange behaviors I'm experiencing please ?

thanks for the help

[letsfindaway]

During Import I see three phases

• the actual import
• loading of the preview. This indeed seems to happen multiple times, and here I see a problem.
• refreshing
• actualization of the document mode (very fast).
• again loading of preview.
• and again actualization of the document mode. During this phase, memory consumption grows

The sequence of these phases is strange. Let me see what triggers it. Memory consumption is low during the first and second phase and grows by about 8 GByte during the third phase. Not a problem for my machine. I assume this are not the scenes, but the rendered PDF images which are cached in the PDFRenderer. See "Caching" in https://github.com/letsfindaway/OpenBoard/discussions/183.

The complete process is finished in less than 1 minute. Here is a video of the import. The test happened with Qt 5 and current dev.

vokoscreenNG-2024-09-30_21-46-58.zip

Please download, unzip and play e.g. in VLC player. GitHub does not support that video format natively.

With Qt6, I get a crash:

ASSERT: "!readjusted || (where == QArrayData::GrowsAtBeginning && freeSpaceAtBegin() >= n) || (where == QArrayData::GrowsAtEnd && freeSpaceAtEnd() >= n)" in file /usr/include/qt6/QtCore/qarraydatapointer.h, line 199

or another time

ASSERT: "sourceCopyConstruct == 1" in file /usr/include/qt6/QtCore/qarraydataops.h, line 508

This happens when adding a new scene to the UBPersistenceWorker queue - will have to analyze this further. During a third try however it went without crash.

I can confirm that the metadata is much too often written, the log states twice per page. This should be improved. I will see what triggers that.

When I switch to board mode after loading the behavior is quite fine. All thumbnails are filled with the PNG previews and I can quite fast go to any page.

So the points I will analyze further:

• What are the five phases?
• Why behaves the second (fourth) phase so strange?
• What triggers writing of metadata?
• Why are there sporadic crashes with Qt 6?

[kaamui]

I have the same behavior except for this document where refreshing loops more than 2 times and memory grows until app crashes.

I looked at the code and the issue with metadata being persisted too many times is because it is called in persistDocumentScene. Changing this is not trivial because I must ensure that it will be called everytimes it needs to be called..

But I don't see why the "loading thumbnail" and "refresh documents thumbnail view would go in a infinite loop, only when document is such heavy. I don't expérience the issue with another document.

[kaamui]

I also had a random crash on Ubuntu 20.04 with Qt 6.7.2 => malloc(): smallbin double linked list corrupted (with branch at 1.7.2a-240815 commit)

Found this on SO :

• https://stackoverflow.com/a/19092698/4759678
• https://stackoverflow.com/a/40787992/4759678

[letsfindaway]

More tests:

• Changing PageCacheSize=2500 disables deletion of scenes, but does not otherwise change the behavior.

For the phases:

• There is actually a phase before the first which I have missed. It is quite fast, it is the loop in UBImportPDF::import() looping over all pages. This is called from UBDocumentManager::importFile(). Here a list of UBGraphicsItem is created.
• The second phase, which takes the most time for me, is creating the scenes. Everythng is fine even in this phase.

Phase three

The third phase is from the Document Thumbnails View, which is loading the PNG images of the pages for the document view. It is triggered because the document controller calls selectDocument() after the import. In detail:

• selectDocument sets the document as current document in the document tree view.
• This triggers UBDocumentController::TreeViewSelectionChanged().
• This again calls UBDocumentController::setDocument().
• This calls UBDocumentContainer::reloadThumbnails()
• which emits documentThumbnailsUpdated
• which is connected to UBDocumentController:: refreshDocumentThumbnailsView()
• and here we have the loop of "Refreshing Document Thumbnails View".

To be continued. Have to go with the dogs now

• The next phase refreshing the thumbnails is also triggered by selectDocument.

For saving the metadata, I wopuld propose to add an additional saveMetadata flag to the signature of persistDocumentScene which defaults to true. We can then set it to false in the import loop.

[kaamui]

For saving the metadata, I wopuld propose to add an additional saveMetadata flag to the signature of persistDocumentScene which defaults to true. We can then set it to false in the import loop.

Yes I was considering that too yesterday, but went far more concerned by the infinite loading loop of thumbnails I was experiencing and forgot it. I also played with the page cache size (tested =10000 :smile: ) and I was seeing "Preparing to load scene 5889" (for example) on my 2402 pages documents at import (hence the suspicion around cache system.

[kaamui]

Please note that I don't experience the loading thumbnails loop with branch at 1.7.2-240815 but it is systematic on current dev. Had one time the loop stop and the thumbnails appear, but most of the time it ends with a crash because of memory growth (I have 8Go and a 16Go swap file)

[kaamui]

A part of the issues comes with the call of initThumbnailsRequired in setDocument that is performed at least twice for UBBoardController.

edit : even for the DocumentController, the fact that reloadThumbnails is called in setDocument is part of the issue.

During an import, loading thumbnails is called

• for document mode when setDocument is called, by calling reload that ends in another refreshDocumentThumbnailsView call
• for board mode, twice, since I refactored reloadThumbnails in boardController to call for initThumbnailsRequired signal but didn't remove the direct call from setDocument
• at the end of the import
• and in refreshDocumentThumbnailsView, I call for UBThumnailAdaptor::load every times. It was maybe valid when I implemented it but now the code changed so much that it probably needs to be rethought (or at least refresh should not be called so many times, especially if the current document thumbnails are already loaded)

[letsfindaway]

Please note that I don't experience the loading thumbnails loop with branch at 1.7.2-240815 but it is systematic on current dev. Had one time the loop stop and the thumbnails appear, but most of the time it ends with a crash because of memory growth (I have 8Go and a 16Go swap file)

I can offer to make a git bisect this afternoon to find the commit causing this.

[kaamui]

It always finishes with display of the thumbnails this morning : it was probably not "infinite" yesterday either, just it crashed after many times of it being performed, because it was the end of the day and I had many things opened in the computer. But still strange that reload after reload momory would grow instead of being constant (at the end) because destructing thumbnails while creating new ones.

Most of the issue is related to the thumbnails handling, that has become terrible at this point.

I also experienced lags on a debian VM because of the displayed messages themselves (!). Commenting them make things way quicker on extreme cases like here.

I'll make quick changes because I can't go on a complete rework with the release of the 1.7.2 being so close, but it feels like a complete rework of the thumbnails management is needed. Maybe by introducing a new dedicated class or transferring the responsibility to an existing one.

[letsfindaway]

It always finishes with display of the thumbnails this morning : it was probably not "infinite" yesterday either, just it crashed after many times of it being performed, because it was the end of the day and I had many things opened in the computer. But still strange that reload after reload momory would grow instead of being constant (at the end) because destructing thumbnails while creating new ones.

I think the memory growth is from the PDFRenderer. Its cache of rendered images only grows and never shrinks

Most of the issue is related to the thumbnails handling, that has become terrible at this point.

Agreed.

I also experienced lags on a debian VM because of the displayed messages themselves (!). Commenting them make things way quicker on extreme cases like here.

I'll make quick changes because I can't go on a complete rework with the release of the 1.7.2 being so close, but it feels like a complete rework of the thumbnails management is needed. Maybe by introducing a new dedicated class or transferring the responsibility to an existing one.

Yep. A rework needs some time and we should take the time. Its mostly not about the algorithms, but about who triggers what and when.

[kaamui]

Another aspect of import is that with a cache at 20 pages, the "removing cache, inserting new one" logic, is called a lot, do you think it could be improved ?

[kaamui]

I think the memory growth is from the PDFRenderer. Its cache of rendered images only grows and never shrinks

Oh ok ! Good to know.

[letsfindaway]

I did a git bisect. The offending commit is 4730dc2cdfe1fae943a820cae201a60c3741d494 and here the line https://github.com/letsfindaway/OpenBoard/blob/6d9fb285e1e88b951f56d95d46679dd243b67626/src/gui/UBBoardThumbnailsView.cpp#L133

Here I want to read the document thumbnails file to initialize the board thumbnail.

But this is done multiple times, because initThumbnails is invoked several times. In contrast to previous commits, this now leads to a message, because the thumbnail image is read using UBThumbnailAdaptor::get(). Previously we did not have this message, but probably initThumbnails was also invoked too often.

During debugging I found at least four calls with quite similar stack frames:

# 1

1   UBBoardThumbnailsView::initThumbnails                   UBBoardThumbnailsView.cpp     168  0x69e127       
2   UBBoardThumbnailsView::qt_static_metacall               moc_UBBoardThumbnailsView.cpp 138  0x49aa91       
3   ??                                                                                         0x7ffff6514fa5 
4   UBDocumentContainer::initThumbnailsRequired             moc_UBDocumentContainer.cpp   310  0x4944e5       
5   UBDocumentContainer::setDocument                        UBDocumentContainer.cpp       51   0x5e339a       
6   UBBoardController::setActiveDocumentScene               UBBoardController.cpp         1624 0x5292d2       
7   UBDocumentController::selectDocument                    UBDocumentController.cpp      2008 0x5f098b       
8   UBDocumentController::importFile                        UBDocumentController.cpp      3191 0x5f7f52       

#2

1   UBBoardThumbnailsView::initThumbnails                   UBBoardThumbnailsView.cpp     168  0x69e127       
2   UBBoardThumbnailsView::qt_static_metacall               moc_UBBoardThumbnailsView.cpp 138  0x49aa91       
3   ??                                                                                         0x7ffff6514fa5 
4   UBDocumentContainer::initThumbnailsRequired             moc_UBDocumentContainer.cpp   310  0x4944e5       
5   UBBoardController::reloadThumbnails                     UBBoardController.cpp         2785 0x53011f       
6   UBDocumentContainer::setDocument                        UBDocumentContainer.cpp       53   0x5e33c9       
7   UBBoardController::setActiveDocumentScene               UBBoardController.cpp         1624 0x5292d2       
8   UBDocumentController::selectDocument                    UBDocumentController.cpp      2008 0x5f098b       
9   UBDocumentController::importFile                        UBDocumentController.cpp      3191 0x5f7f52       

#3

1   UBBoardThumbnailsView::initThumbnails                   UBBoardThumbnailsView.cpp     168  0x69e127       
2   UBBoardThumbnailsView::qt_static_metacall               moc_UBBoardThumbnailsView.cpp 138  0x49aa91       
3   ??                                                                                         0x7ffff6514fa5 
4   UBDocumentContainer::initThumbnailsRequired             moc_UBDocumentContainer.cpp   310  0x4944e5       
5   UBDocumentContainer::setDocument                        UBDocumentContainer.cpp       51   0x5e339a       
6   UBBoardController::setActiveDocumentScene               UBBoardController.cpp         1624 0x5292d2       
7   UBDocumentController::importFile                        UBDocumentController.cpp      3200 0x5f802a       

#4

1   UBBoardThumbnailsView::initThumbnails                   UBBoardThumbnailsView.cpp     168  0x69e127       
2   UBBoardThumbnailsView::qt_static_metacall               moc_UBBoardThumbnailsView.cpp 138  0x49aa91       
3   ??                                                                                         0x7ffff6514fa5 
4   UBDocumentContainer::initThumbnailsRequired             moc_UBDocumentContainer.cpp   310  0x4944e5       
5   UBBoardController::reloadThumbnails                     UBBoardController.cpp         2785 0x53011f       
6   UBDocumentContainer::setDocument                        UBDocumentContainer.cpp       53   0x5e33c9       
7   UBBoardController::setActiveDocumentScene               UBBoardController.cpp         1624 0x5292d2       
8   UBDocumentController::importFile                        UBDocumentController.cpp      3200 0x5f802a       

What you can see:

• importFile calls selectDocument and setActiveDocumentScene.
• However the latter also calls setDocument, so this is at least doubled.
• setDocument calls reloadThumbnails, which emits initThumbnailsRequired.
• setDocument however also emits this signal

This leads to those multiple invocations.

I have now tried to reduce the number of unnecessary refreshes and created branch try-fix-import:

• do nothing in UBBoardController::setActiveDocumentScene if scene does not change.
• do not emit emit initThumbnailsRequired in UBDocumentContainer::setDocument, as it is emitted later anyway.

Could you probably try this branch?

[kaamui]

I'm working on a reducing these calls too ! I'll probably be able to commit my changes today. I propose that you stop there and eventually review my commit when it reaches the server ?

I have a lot more changes but my changes are more open to introduce regressions where the thumbnails are not up-to-date.

[letsfindaway]

ok. just added another commit for duplicate loading of document thumbnails.

But yes, I'm now waiting for your commits.

[letsfindaway]

But in the end I think: it is not the perf-lazy-loading which introduced the problems, it only made them more apparent and heavier in their effects on performance and memory.

[kaamui]

But in the end I think: it is not the perf-lazy-loading which introduced the problems, it only made them more apparent and heavier in their effects on performance and memory.

I totally agree : the core issue comes with the number of cases where a lot of useless calls are performed, multiple times, sometimes heavy ones, because of the poor architecture of the code, as a whole. There's a lot to improve here.

[kaamui]

I pushed my commit, but it is not finished. But it should be a push in the right direction.

I'll make some improvements tomorrow.

Thank you for your help

[letsfindaway]

This happens when adding a new scene to the UBPersistenceWorker queue - will have to analyze this further. During a third try however it went without crash.

I just pushed a third commit 97dfb01dbfbbbb80f90ee80c1d8f04c3d563af9a probably fixing the crash. I tried it several times with Qt 6 and did not observe any problems.

I assume the problem was the saves list in UBPersistenceworker. It is accessed by the main thread, but also by the worker thread. The semaphore does not protect access to that list. It only counts the number of entries and blocks the worker when the list is empty.

In such heavy load scenarios like that big import I can in fact imagine that the saves list is populated with more than one entry and that a new entry is added while on the other side an entry is removed and processed in the worker.

I now introduced a mutex to protect access to the saves list.

[letsfindaway]

Another aspect of import is that with a cache at 20 pages, the "removing cache, inserting new one" logic, is called a lot, do you think it could be improved ?

It is actually not a big thing. Insert and remove from cache is just a operation on a map, which is quite fast. And what are the alternatives?

• We could delete the scenes immediately after we have loaded them and created the thumbnail. This can also be accomplished with a cache site of 1.
• Or we keep all the scenes and delete them never. This can be accomplished with a cache size of 999999.

But if we set the size to 1 or 20 makes no difference. The number of scene creations and deletions is roughly the same.

So the consequence could be that we remove the debug output?

[kaamui]

Hi @letsfindaway

I found that a lot of lags was due to updateThumbnailsPos in UBBoardThumbnailsView, and tried to reduce the amount of calls made of it. Because even with your improvements in page loading, the user could experience a lag of several seconds, just because of the thumbnails position updating... and so even if the left palette was hidden ! Related commit => a88572a5 Don't hesitate to review it and make feedback.

I also found that sometimes after a PDF import the active live thumbnail is not updated (not repainted), as one of things that I broke with all these refactoring. It was already broken with my first commit refactoring thumbnails handling I think. Could look at what's happening and help me on this ?

I also have some issues now with thumbnails placement, that is resolved when resizing. For example, the scrollbar no longer appear instantly when needed, I'm searching why

I'm trying to debug this but I know you're pretty good with this type of issues regarding placement, and juggling with different type of coordinates, so if you have time to look at it, or some advice on why the QGraphicsView is no longer updating correctly your help is welcome.

[letsfindaway]

Hi @letsfindaway

I found that a lot of lags was due to updateThumbnailsPos in UBBoardThumbnailsView, and tried to reduce the amount of calls made of it. Because even with your improvements in page loading, the user could experience a lag of several seconds, just because of the thumbnails position updating... and so even if the left palette was hidden ! Related commit => a88572a Don't hesitate to review it and make feedback.

I also found that sometimes after a PDF import the active live thumbnail is not updated (not repainted), as one of things that I broke with all these refactoring. It was already broken with my first commit refactoring thumbnails handling I think. Could look at what's happening and help me on this ?

There should be some call setting the scene for the thumbnail so that it can update. I tried to set the scene only for the active thumbnail and to set it to nullptr for all others to avoid having too many references to the scene, which would inhibit deleting it.

I also have some issues now with thumbnails placement, that is resolved when resizing. For example, the scrollbar no longer appear instantly when needed, I'm searching why

I'm trying to debug this but I know you're pretty good with this type of issues regarding placement, and juggling with different type of coordinates, so if you have time to look at it, or some advice on why the QGraphicsView is no longer updating correctly your help is welcome.

I don't have much time today and over the weekend as I have a visitor here and we expect our son coming back from Japan today and staying over the weekend.

[kaamui]

I don't have much time today and over the weekend as I have a visitor here and we expect our son coming back from Japan today and staying over the weekend

No problem, you already do a lot for the project in your free time ! I'll try to find out what's happening myself today

[kaamui]

found for the thumbnails placement ! => 09d2d127e0

[kaamui]

There should be some call setting the scene for the thumbnail so that it can update. I tried to set the scene only for the active thumbnail and to set it to nullptr for all others to avoid having too many references to the scene, which would inhibit deleting it.

And fixed => 81582ea10

[letsfindaway]

I think this is now fixed with the current dev. Documents with many pages as well as heavy pages with ten thousands of strokes are now loading quite smoothly.