Re: [Cin] Complete system hangs (likely out of memory)
пн, 23 янв. 2023 г., 18:45 Stefan de Konink <[email protected]>:
Hi Phyllis,
With the suggestion of Richard Nolde, I decided to do a day of work in Cinerella with heaptrack enabled in the background. It was a pleasant surprise how well it performed. (And no crashes ;)
And I am also surprised how functional Heaptrack itself has been designed, I'll forward this application to my peers. To screenshots attached.
Thanks for being curious, Stephan! I looked at attached screenshots and I found it strange how Peaked and Leaked basically stays very near each other for each function line?
If anyone is intested in my heaptrack file, I can obviously make it available. But I would really recommend adding this tool to your toolbelt!
-- Stefan
On Monday, January 23, 2023 4:55:08 PM CET, Andrew Randrianasulu wrote:
I looked at attached screenshots and I found it strange how Peaked and Leaked basically stays very near each other for each function line?
I cannot comment on that part. But I did some more digging. The 'true' leaks start upon the moment that the viewer is actually invoked. It does not happen when the resources are loaded or the images get thumbnailed, or with the 'low res preview' when using mouse over. For each opening (of the same file of about 3GB) the leak increases 70MB. It happens when an actual new window is opened, but also when just using "View" which opens the same file in the same window. A few scratches in a single view, can bring this view to a higher leak. Ultimately it seems to be something under VRender::run() with both audio and video VRender::process_buffer() andARender::process_buffer(). Without going into the code: is there any way that can disable the cache? Setting it to 0MB in preferences does not seem to have an effect. -- Stefan
Stefan, Maybe *Andrew *knows a way to disable the cache, but I do not. I have downloaded a Nikon d3200 video file and absolutely see the problem, but also see the problem with other videos. RIght now I am going to test CinGG earlier versions to see when the problem started. It will take awhile. On Mon, Jan 23, 2023 at 12:10 PM Stefan de Konink <[email protected]> wrote:
On Monday, January 23, 2023 4:55:08 PM CET, Andrew Randrianasulu wrote:
I looked at attached screenshots and I found it strange how Peaked and Leaked basically stays very near each other for each function line?
I cannot comment on that part. But I did some more digging.
The 'true' leaks start upon the moment that the viewer is actually invoked. It does not happen when the resources are loaded or the images get thumbnailed, or with the 'low res preview' when using mouse over.
For each opening (of the same file of about 3GB) the leak increases 70MB. It happens when an actual new window is opened, but also when just using "View" which opens the same file in the same window.
A few scratches in a single view, can bring this view to a higher leak. Ultimately it seems to be something under VRender::run() with both audio and video VRender::process_buffer() andARender::process_buffer().
Without going into the code: is there any way that can disable the cache? Setting it to 0MB in preferences does not seem to have an effect.
-- Stefan
On Monday, January 23, 2023 8:39:26 PM CET, Phyllis Smith wrote:
Stefan, Maybe Andrew knows a way to disable the cache, but I do not. I have downloaded a Nikon d3200 video file and absolutely see the problem, but also see the problem with other videos. RIght now I am going to test CinGG earlier versions to see when the problem started. It will take awhile.
I am currently digging too. At this moment my guess is that the garbage collection itself does not unlock 'something'. I don't know if it is the "EDL" or "Indexable", which I am now proactively walking through via gdb. I'll keep you posted. -- Stefan
пн, 23 янв. 2023 г., 22:10 Stefan de Konink <[email protected]>:
On Monday, January 23, 2023 4:55:08 PM CET, Andrew Randrianasulu wrote:
I looked at attached screenshots and I found it strange how Peaked and Leaked basically stays very near each other for each function line?
I cannot comment on that part. But I did some more digging.
The 'true' leaks start upon the moment that the viewer is actually invoked. It does not happen when the resources are loaded or the images get thumbnailed, or with the 'low res preview' when using mouse over.
For each opening (of the same file of about 3GB) the leak increases 70MB. It happens when an actual new window is opened, but also when just using "View" which opens the same file in the same window.
A few scratches in a single view, can bring this view to a higher leak. Ultimately it seems to be something under VRender::run() with both audio and video VRender::process_buffer() andARender::process_buffer().
Without going into the code: is there any way that can disable the cache?
sorry, I can't think about anything BUT disabling it by early return in functions allocating cache items ...
Setting it to 0MB in preferences does not seem to have an effect.
-- Stefan
On Monday, January 23, 2023 10:56:00 PM CET, Andrew Randrianasulu wrote:
sorry, I can't think about anything BUT disabling it by early return in functions allocating cache items ...
I added a -1, as effective. Lets say the solution is the file should be closed, but I am not yet at the point what either "does not do that" (mwindow/vwindow) or "is logically wrong" (garbage collector). -- Stefan
The bug could be due to ffmpeg 5.1 as opposed to ffmpeg 4.x applied patches or some other change. Valgrind (heaptrack is really great but I am too lazy to git it) on Oct 31, 2020 does not exhibit this astronomical loss of available memory. October 31, 2020 GIT ==257884== definitely lost: 256 bytes in 8 blocks ==257884== *indirectly lost: 64 bytes* in 8 blocks ==257884== possibly lost: 0 bytes in 0 blocks Current GIT: ==4985== definitely lost: 1,296 bytes in 9 blocks ==4985== * indirectly lost: 1,626,411 bytes* in 272 blocks ==4985== possibly lost: 528,288 bytes in 14 blocks On Mon, Jan 23, 2023 at 3:01 PM Stefan de Konink <[email protected]> wrote:
On Monday, January 23, 2023 10:56:00 PM CET, Andrew Randrianasulu wrote:
sorry, I can't think about anything BUT disabling it by early return in functions allocating cache items ...
I added a -1, as effective.
Lets say the solution is the file should be closed, but I am not yet at the point what either "does not do that" (mwindow/vwindow) or "is logically wrong" (garbage collector).
-- Stefan
Hi Andrew, When changing this, to be more in line with the "HV" code... diff --git a/cinelerra-5.1/cinelerra/playbackengine.C b/cinelerra-5.1/cinelerra/playbackengine.C index 815e506f..3c2a9368 100644 --- a/cinelerra-5.1/cinelerra/playbackengine.C +++ b/cinelerra-5.1/cinelerra/playbackengine.C @@ -172,9 +172,9 @@ void PlaybackEngine::create_cache() { cache_lock->lock("PlaybackEngine::create_cache"); if( audio_cache ) - audio_cache->remove_user(); + delete audio_cache; if( video_cache ) - video_cache->remove_user(); + delete video_cache; audio_cache = new CICache(preferences); video_cache = new CICache(preferences); cache_lock->unlock(); ...will result in a fix for the memory leak, but does cause a warning message. Garbage::~Garbage: title=CICache users=1 was not deleted by Garbage::remove_user This was changed with f5725c7e (7/4/20). But also shows that many unrelated changes cause real issues to hide. I found this by hand, not even using git bisect. I understand the urge for the garbage collector, but for some reason not behaving. What I notice is that the 'last' object is empty when it is actually destroyed by the garbage collector (~CICache). What is the best way going forward? -- Stefan
вт, 24 янв. 2023 г., 02:44 Stefan de Konink <[email protected]>:
Hi Andrew,
When changing this, to be more in line with the "HV" code...
diff --git a/cinelerra-5.1/cinelerra/playbackengine.C b/cinelerra-5.1/cinelerra/playbackengine.C index 815e506f..3c2a9368 100644 --- a/cinelerra-5.1/cinelerra/playbackengine.C +++ b/cinelerra-5.1/cinelerra/playbackengine.C @@ -172,9 +172,9 @@ void PlaybackEngine::create_cache() { cache_lock->lock("PlaybackEngine::create_cache"); if( audio_cache ) - audio_cache->remove_user(); + delete audio_cache; if( video_cache ) - video_cache->remove_user(); + delete video_cache; audio_cache = new CICache(preferences); video_cache = new CICache(preferences); cache_lock->unlock();
...will result in a fix for the memory leak, but does cause a warning message.
Garbage::~Garbage: title=CICache users=1 was not deleted by Garbage::remove_user
This was changed with f5725c7e (7/4/20). But also shows that many unrelated changes cause real issues to hide. I found this by hand, not even using git bisect. I understand the urge for the garbage collector, but for some reason not behaving. What I notice is that the 'last' object is empty when it is actually destroyed by the garbage collector (~CICache). What is the best way going forward?
sorry, I am not programmer enough for answering this .. but you tried to leave both lines there? also, I did grep remove_user cinelerra/* and among results cinelerra/mwindow.C: if( audio_cache ) { audio_cache->remove_user(); audio_cache = 0; } cinelerra/mwindow.C: if( video_cache ) { video_cache->remove_user(); video_cache = 0; } [..] cinelerra/fileref.C: if( acache ) { acache->remove_user(); acache = 0; } cinelerra/fileref.C: if( vcache ) { vcache->remove_user(); vcache = 0; } so may be nulling it after remove_user actually intended?
вт, 24 янв. 2023 г., 02:44 Stefan de Konink <[email protected]>:
Hi Andrew,
When changing this, to be more in line with the "HV" code...
diff --git a/cinelerra-5.1/cinelerra/playbackengine.C b/cinelerra-5.1/cinelerra/playbackengine.C index 815e506f..3c2a9368 100644 --- a/cinelerra-5.1/cinelerra/playbackengine.C +++ b/cinelerra-5.1/cinelerra/playbackengine.C @@ -172,9 +172,9 @@ void PlaybackEngine::create_cache() { cache_lock->lock("PlaybackEngine::create_cache"); if( audio_cache ) - audio_cache->remove_user(); + delete audio_cache; if( video_cache ) - video_cache->remove_user(); + delete video_cache; audio_cache = new CICache(preferences); video_cache = new CICache(preferences); cache_lock->unlock();
...will result in a fix for the memory leak, but does cause a warning message.
Garbage::~Garbage: title=CICache users=1 was not deleted by Garbage::remove_user
This was changed with f5725c7e (7/4/20).
https://git.cinelerra-gg.org/git/?p=goodguy/cinelerra.git;a=blobdiff;f=cinel... hm .... may be you also can try to restore this idea about calling new only if cache does not exist ... @@ -162,10 +164,12 @@ void PlaybackEngine::wait_render_engine() void PlaybackEngine::create_cache() { - if(audio_cache) { delete audio_cache; audio_cache = 0; } - if(video_cache) { delete video_cache; video_cache = 0; } - if(!audio_cache) audio_cache = new CICache(preferences); - if(!video_cache) video_cache = new CICache(preferences); + if( audio_cache ) + audio_cache->remove_user(); + if( video_cache ) + video_cache->remove_user(); + audio_cache = new CICache(preferences); + video_cache = new CICache(preferences); } But also shows that many unrelated
changes cause real issues to hide. I found this by hand, not even using git bisect. I understand the urge for the garbage collector, but for some reason not behaving. What I notice is that the 'last' object is empty when it is actually destroyed by the garbage collector (~CICache). What is the best way going forward?
-- Stefan
Andrew, I found where the problem was introduced. Dec. 6, 2022 GIT is good. 3c7c8a08800c3e100388996f0e2c2eea9761ebe1 Dec.27, 2022 GIT is bad. 175a7314e8e927128787feeb7ba5f42530f0a319 On Mon, Jan 23, 2023 at 7:51 PM Andrew Randrianasulu < [email protected]> wrote:
вт, 24 янв. 2023 г., 02:44 Stefan de Konink <[email protected]>:
Hi Andrew,
When changing this, to be more in line with the "HV" code...
diff --git a/cinelerra-5.1/cinelerra/playbackengine.C b/cinelerra-5.1/cinelerra/playbackengine.C index 815e506f..3c2a9368 100644 --- a/cinelerra-5.1/cinelerra/playbackengine.C +++ b/cinelerra-5.1/cinelerra/playbackengine.C @@ -172,9 +172,9 @@ void PlaybackEngine::create_cache() { cache_lock->lock("PlaybackEngine::create_cache"); if( audio_cache ) - audio_cache->remove_user(); + delete audio_cache; if( video_cache ) - video_cache->remove_user(); + delete video_cache; audio_cache = new CICache(preferences); video_cache = new CICache(preferences); cache_lock->unlock();
...will result in a fix for the memory leak, but does cause a warning message.
Garbage::~Garbage: title=CICache users=1 was not deleted by Garbage::remove_user
This was changed with f5725c7e (7/4/20).
https://git.cinelerra-gg.org/git/?p=goodguy/cinelerra.git;a=blobdiff;f=cinel...
hm ....
may be you also can try to restore this idea about calling new only if cache does not exist ...
@@ -162,10 +164,12 @@ void PlaybackEngine::wait_render_engine()
void PlaybackEngine::create_cache() { - if(audio_cache) { delete audio_cache; audio_cache = 0; } - if(video_cache) { delete video_cache; video_cache = 0; } - if(!audio_cache) audio_cache = new CICache(preferences); - if(!video_cache) video_cache = new CICache(preferences); + if( audio_cache ) + audio_cache->remove_user(); + if( video_cache ) + video_cache->remove_user(); + audio_cache = new CICache(preferences); + video_cache = new CICache(preferences); }
But also shows that many unrelated
changes cause real issues to hide. I found this by hand, not even using git bisect. I understand the urge for the garbage collector, but for some reason not behaving. What I notice is that the 'last' object is empty when it is actually destroyed by the garbage collector (~CICache). What is the best way going forward?
-- Stefan
Andrew, I did get a little confused by part 1 and part 2 of that checkin. Did I get it wrong? On Mon, Jan 23, 2023 at 8:26 PM Phyllis Smith <[email protected]> wrote:
Andrew, I found where the problem was introduced. Dec. 6, 2022 GIT is good. 3c7c8a08800c3e100388996f0e2c2eea9761ebe1 Dec.27, 2022 GIT is bad. 175a7314e8e927128787feeb7ba5f42530f0a319
On Mon, Jan 23, 2023 at 7:51 PM Andrew Randrianasulu < [email protected]> wrote:
вт, 24 янв. 2023 г., 02:44 Stefan de Konink <[email protected]>:
Hi Andrew,
When changing this, to be more in line with the "HV" code...
diff --git a/cinelerra-5.1/cinelerra/playbackengine.C b/cinelerra-5.1/cinelerra/playbackengine.C index 815e506f..3c2a9368 100644 --- a/cinelerra-5.1/cinelerra/playbackengine.C +++ b/cinelerra-5.1/cinelerra/playbackengine.C @@ -172,9 +172,9 @@ void PlaybackEngine::create_cache() { cache_lock->lock("PlaybackEngine::create_cache"); if( audio_cache ) - audio_cache->remove_user(); + delete audio_cache; if( video_cache ) - video_cache->remove_user(); + delete video_cache; audio_cache = new CICache(preferences); video_cache = new CICache(preferences); cache_lock->unlock();
...will result in a fix for the memory leak, but does cause a warning message.
Garbage::~Garbage: title=CICache users=1 was not deleted by Garbage::remove_user
This was changed with f5725c7e (7/4/20).
https://git.cinelerra-gg.org/git/?p=goodguy/cinelerra.git;a=blobdiff;f=cinel...
hm ....
may be you also can try to restore this idea about calling new only if cache does not exist ...
@@ -162,10 +164,12 @@ void PlaybackEngine::wait_render_engine()
void PlaybackEngine::create_cache() { - if(audio_cache) { delete audio_cache; audio_cache = 0; } - if(video_cache) { delete video_cache; video_cache = 0; } - if(!audio_cache) audio_cache = new CICache(preferences); - if(!video_cache) video_cache = new CICache(preferences); + if( audio_cache ) + audio_cache->remove_user(); + if( video_cache ) + video_cache->remove_user(); + audio_cache = new CICache(preferences); + video_cache = new CICache(preferences); }
But also shows that many unrelated
changes cause real issues to hide. I found this by hand, not even using git bisect. I understand the urge for the garbage collector, but for some reason not behaving. What I notice is that the 'last' object is empty when it is actually destroyed by the garbage collector (~CICache). What is the best way going forward?
-- Stefan
вт, 24 янв. 2023 г., 06:28 Phyllis Smith <[email protected]>:
Andrew, I did get a little confused by part 1 and part 2 of that checkin. Did I get it wrong?
no ... I hope. does removing line I introduced total_lock->unlock(); help with leak?
On Mon, Jan 23, 2023 at 8:26 PM Phyllis Smith <[email protected]> wrote:
Andrew, I found where the problem was introduced. Dec. 6, 2022 GIT is good. 3c7c8a08800c3e100388996f0e2c2eea9761ebe1 Dec.27, 2022 GIT is bad. 175a7314e8e927128787feeb7ba5f42530f0a319
On Mon, Jan 23, 2023 at 7:51 PM Andrew Randrianasulu < [email protected]> wrote:
вт, 24 янв. 2023 г., 02:44 Stefan de Konink <[email protected]>:
Hi Andrew,
When changing this, to be more in line with the "HV" code...
diff --git a/cinelerra-5.1/cinelerra/playbackengine.C b/cinelerra-5.1/cinelerra/playbackengine.C index 815e506f..3c2a9368 100644 --- a/cinelerra-5.1/cinelerra/playbackengine.C +++ b/cinelerra-5.1/cinelerra/playbackengine.C @@ -172,9 +172,9 @@ void PlaybackEngine::create_cache() { cache_lock->lock("PlaybackEngine::create_cache"); if( audio_cache ) - audio_cache->remove_user(); + delete audio_cache; if( video_cache ) - video_cache->remove_user(); + delete video_cache; audio_cache = new CICache(preferences); video_cache = new CICache(preferences); cache_lock->unlock();
...will result in a fix for the memory leak, but does cause a warning message.
Garbage::~Garbage: title=CICache users=1 was not deleted by Garbage::remove_user
This was changed with f5725c7e (7/4/20).
https://git.cinelerra-gg.org/git/?p=goodguy/cinelerra.git;a=blobdiff;f=cinel...
hm ....
may be you also can try to restore this idea about calling new only if cache does not exist ...
@@ -162,10 +164,12 @@ void PlaybackEngine::wait_render_engine()
void PlaybackEngine::create_cache() { - if(audio_cache) { delete audio_cache; audio_cache = 0; } - if(video_cache) { delete video_cache; video_cache = 0; } - if(!audio_cache) audio_cache = new CICache(preferences); - if(!video_cache) video_cache = new CICache(preferences); + if( audio_cache ) + audio_cache->remove_user(); + if( video_cache ) + video_cache->remove_user(); + audio_cache = new CICache(preferences); + video_cache = new CICache(preferences); }
But also shows that many unrelated
changes cause real issues to hide. I found this by hand, not even using git bisect. I understand the urge for the garbage collector, but for some reason not behaving. What I notice is that the 'last' object is empty when it is actually destroyed by the garbage collector (~CICache). What is the best way going forward?
-- Stefan
Andrew, no - removing that line does not help leak so it must be the "if parent". It takes awhile to test but will revert from current code of: if( users == 1 ) { remove_user(); total_lock->unlock(); return 0 } //printf("users: %i \n", users ); EDL *parent - edl->parent_edl; if(parent) remove_user(); To: if( users == 1 ) { remove_user(); return 0 } remove_user(); Better to have EDL undo problem then run out of memory. On Mon, Jan 23, 2023 at 8:31 PM Andrew Randrianasulu < [email protected]> wrote:
вт, 24 янв. 2023 г., 06:28 Phyllis Smith <[email protected]>:
Andrew, I did get a little confused by part 1 and part 2 of that checkin. Did I get it wrong?
no ... I hope.
does removing line I introduced
total_lock->unlock();
help with leak?
On Mon, Jan 23, 2023 at 8:26 PM Phyllis Smith <[email protected]> wrote:
Andrew, I found where the problem was introduced. Dec. 6, 2022 GIT is good. 3c7c8a08800c3e100388996f0e2c2eea9761ebe1 Dec.27, 2022 GIT is bad. 175a7314e8e927128787feeb7ba5f42530f0a319
On Mon, Jan 23, 2023 at 7:51 PM Andrew Randrianasulu < [email protected]> wrote:
вт, 24 янв. 2023 г., 02:44 Stefan de Konink <[email protected]>:
Hi Andrew,
When changing this, to be more in line with the "HV" code...
diff --git a/cinelerra-5.1/cinelerra/playbackengine.C b/cinelerra-5.1/cinelerra/playbackengine.C index 815e506f..3c2a9368 100644 --- a/cinelerra-5.1/cinelerra/playbackengine.C +++ b/cinelerra-5.1/cinelerra/playbackengine.C @@ -172,9 +172,9 @@ void PlaybackEngine::create_cache() { cache_lock->lock("PlaybackEngine::create_cache"); if( audio_cache ) - audio_cache->remove_user(); + delete audio_cache; if( video_cache ) - video_cache->remove_user(); + delete video_cache; audio_cache = new CICache(preferences); video_cache = new CICache(preferences); cache_lock->unlock();
...will result in a fix for the memory leak, but does cause a warning message.
Garbage::~Garbage: title=CICache users=1 was not deleted by Garbage::remove_user
This was changed with f5725c7e (7/4/20).
https://git.cinelerra-gg.org/git/?p=goodguy/cinelerra.git;a=blobdiff;f=cinel...
hm ....
may be you also can try to restore this idea about calling new only if cache does not exist ...
@@ -162,10 +164,12 @@ void PlaybackEngine::wait_render_engine()
void PlaybackEngine::create_cache() { - if(audio_cache) { delete audio_cache; audio_cache = 0; } - if(video_cache) { delete video_cache; video_cache = 0; } - if(!audio_cache) audio_cache = new CICache(preferences); - if(!video_cache) video_cache = new CICache(preferences); + if( audio_cache ) + audio_cache->remove_user(); + if( video_cache ) + video_cache->remove_user(); + audio_cache = new CICache(preferences); + video_cache = new CICache(preferences); }
But also shows that many unrelated
changes cause real issues to hide. I found this by hand, not even using git bisect. I understand the urge for the garbage collector, but for some reason not behaving. What I notice is that the 'last' object is empty when it is actually destroyed by the garbage collector (~CICache). What is the best way going forward?
-- Stefan
вт, 24 янв. 2023 г., 07:11 Phyllis Smith <[email protected]>:
Andrew, no - removing that line does not help leak so it must be the "if parent". It takes awhile to test but will revert from current code of: if( users == 1 ) { remove_user(); total_lock->unlock(); return 0 }
//printf("users: %i \n", users ); EDL *parent - edl->parent_edl; if(parent) remove_user();
To: if( users == 1 ) { remove_user(); return 0 } remove_user();
Better to have EDL undo problem then run out of memory.
hm, may be I'll come with some additional line (like removing this *parent link) but can you just comment it out with // rem: it leask added? also, I thought Stefan was chasing earlier leak, may be independent from this one
On Mon, Jan 23, 2023 at 8:31 PM Andrew Randrianasulu < [email protected]> wrote:
вт, 24 янв. 2023 г., 06:28 Phyllis Smith <[email protected]>:
Andrew, I did get a little confused by part 1 and part 2 of that checkin. Did I get it wrong?
no ... I hope.
does removing line I introduced
total_lock->unlock();
help with leak?
On Mon, Jan 23, 2023 at 8:26 PM Phyllis Smith <[email protected]> wrote:
Andrew, I found where the problem was introduced. Dec. 6, 2022 GIT is good. 3c7c8a08800c3e100388996f0e2c2eea9761ebe1 Dec.27, 2022 GIT is bad. 175a7314e8e927128787feeb7ba5f42530f0a319
On Mon, Jan 23, 2023 at 7:51 PM Andrew Randrianasulu < [email protected]> wrote:
вт, 24 янв. 2023 г., 02:44 Stefan de Konink <[email protected]>:
Hi Andrew,
When changing this, to be more in line with the "HV" code...
diff --git a/cinelerra-5.1/cinelerra/playbackengine.C b/cinelerra-5.1/cinelerra/playbackengine.C index 815e506f..3c2a9368 100644 --- a/cinelerra-5.1/cinelerra/playbackengine.C +++ b/cinelerra-5.1/cinelerra/playbackengine.C @@ -172,9 +172,9 @@ void PlaybackEngine::create_cache() { cache_lock->lock("PlaybackEngine::create_cache"); if( audio_cache ) - audio_cache->remove_user(); + delete audio_cache; if( video_cache ) - video_cache->remove_user(); + delete video_cache; audio_cache = new CICache(preferences); video_cache = new CICache(preferences); cache_lock->unlock();
...will result in a fix for the memory leak, but does cause a warning message.
Garbage::~Garbage: title=CICache users=1 was not deleted by Garbage::remove_user
This was changed with f5725c7e (7/4/20).
https://git.cinelerra-gg.org/git/?p=goodguy/cinelerra.git;a=blobdiff;f=cinel...
hm ....
may be you also can try to restore this idea about calling new only if cache does not exist ...
@@ -162,10 +164,12 @@ void PlaybackEngine::wait_render_engine()
void PlaybackEngine::create_cache() { - if(audio_cache) { delete audio_cache; audio_cache = 0; } - if(video_cache) { delete video_cache; video_cache = 0; } - if(!audio_cache) audio_cache = new CICache(preferences); - if(!video_cache) video_cache = new CICache(preferences); + if( audio_cache ) + audio_cache->remove_user(); + if( video_cache ) + video_cache->remove_user(); + audio_cache = new CICache(preferences); + video_cache = new CICache(preferences); }
But also shows that many unrelated
changes cause real issues to hide. I found this by hand, not even using git bisect. I understand the urge for the garbage collector, but for some reason not behaving. What I notice is that the 'last' object is empty when it is actually destroyed by the garbage collector (~CICache). What is the best way going forward?
-- Stefan
Andrew, I think it is the same leak. Leak is gone when I changed the code back. I will test the one Nikon d3200.mp4 video I have. But after that I will have to stop for today due to being too tired. On Mon, Jan 23, 2023 at 9:16 PM Andrew Randrianasulu < [email protected]> wrote:
вт, 24 янв. 2023 г., 07:11 Phyllis Smith <[email protected]>:
Andrew, no - removing that line does not help leak so it must be the "if parent". It takes awhile to test but will revert from current code of: if( users == 1 ) { remove_user(); total_lock->unlock(); return 0 }
//printf("users: %i \n", users ); EDL *parent - edl->parent_edl; if(parent) remove_user();
To: if( users == 1 ) { remove_user(); return 0 } remove_user();
Better to have EDL undo problem then run out of memory.
hm, may be I'll come with some additional line (like removing this *parent link) but can you just comment it out with
// rem: it leask
added?
also, I thought Stefan was chasing earlier leak, may be independent from this one
On Mon, Jan 23, 2023 at 8:31 PM Andrew Randrianasulu < [email protected]> wrote:
вт, 24 янв. 2023 г., 06:28 Phyllis Smith <[email protected]>:
Andrew, I did get a little confused by part 1 and part 2 of that checkin. Did I get it wrong?
no ... I hope.
does removing line I introduced
total_lock->unlock();
help with leak?
On Mon, Jan 23, 2023 at 8:26 PM Phyllis Smith <[email protected]> wrote:
Andrew, I found where the problem was introduced. Dec. 6, 2022 GIT is good. 3c7c8a08800c3e100388996f0e2c2eea9761ebe1 Dec.27, 2022 GIT is bad. 175a7314e8e927128787feeb7ba5f42530f0a319
On Mon, Jan 23, 2023 at 7:51 PM Andrew Randrianasulu < [email protected]> wrote:
вт, 24 янв. 2023 г., 02:44 Stefan de Konink <[email protected]>:
> Hi Andrew, > > When changing this, to be more in line with the "HV" code... > > diff --git a/cinelerra-5.1/cinelerra/playbackengine.C > b/cinelerra-5.1/cinelerra/playbackengine.C > index 815e506f..3c2a9368 100644 > --- a/cinelerra-5.1/cinelerra/playbackengine.C > +++ b/cinelerra-5.1/cinelerra/playbackengine.C > @@ -172,9 +172,9 @@ void PlaybackEngine::create_cache() > { > cache_lock->lock("PlaybackEngine::create_cache"); > if( audio_cache ) > - audio_cache->remove_user(); > + delete audio_cache; > if( video_cache ) > - video_cache->remove_user(); > + delete video_cache; > audio_cache = new CICache(preferences); > video_cache = new CICache(preferences); > cache_lock->unlock(); > > > ...will result in a fix for the memory leak, but does cause a > warning > message. > > Garbage::~Garbage: title=CICache users=1 was not deleted by > Garbage::remove_user > > > This was changed with f5725c7e (7/4/20).
https://git.cinelerra-gg.org/git/?p=goodguy/cinelerra.git;a=blobdiff;f=cinel...
hm ....
may be you also can try to restore this idea about calling new only if cache does not exist ...
@@ -162,10 +164,12 @@ void PlaybackEngine::wait_render_engine()
void PlaybackEngine::create_cache() { - if(audio_cache) { delete audio_cache; audio_cache = 0; } - if(video_cache) { delete video_cache; video_cache = 0; } - if(!audio_cache) audio_cache = new CICache(preferences); - if(!video_cache) video_cache = new CICache(preferences); + if( audio_cache ) + audio_cache->remove_user(); + if( video_cache ) + video_cache->remove_user(); + audio_cache = new CICache(preferences); + video_cache = new CICache(preferences); }
But also shows that many unrelated > changes cause real issues to hide. I found this by hand, not even > using git > bisect. I understand the urge for the garbage collector, but for > some > reason not behaving. What I notice is that the 'last' object is > empty when > it is actually destroyed by the garbage collector (~CICache). What > is the > best way going forward? > > -- > Stefan >
вт, 24 янв. 2023 г., 07:24 Phyllis Smith <[email protected]>:
Andrew, I think it is the same leak. Leak is gone when I changed the code back. I will test the one Nikon d3200.mp4 video I have.
I tried heaptrack from git and it seems to detect few leaks (over run cingg, load video, open view from its context menu in resources few times, quit) , some of them large, as in few megabytes each. tracking file is 3.6 mb compressed itself 3,6M /home/guest/heaptrack.cin.2457.zst but processed/compressed by heaptrack/build/bin/heaptrack_print ~/heap*cin*.zst -l > ~/heaptrack-processed.log xz -9 is only 8kb. So , attached. so .... lets sleep over this problem a bit, hopefully we'll find solution for at least recent regression
But after that I will have to stop for today due to being too tired.
On Mon, Jan 23, 2023 at 9:16 PM Andrew Randrianasulu < [email protected]> wrote:
вт, 24 янв. 2023 г., 07:11 Phyllis Smith <[email protected]>:
Andrew, no - removing that line does not help leak so it must be the "if parent". It takes awhile to test but will revert from current code of: if( users == 1 ) { remove_user(); total_lock->unlock(); return 0 }
//printf("users: %i \n", users ); EDL *parent - edl->parent_edl; if(parent) remove_user();
To: if( users == 1 ) { remove_user(); return 0 } remove_user();
Better to have EDL undo problem then run out of memory.
hm, may be I'll come with some additional line (like removing this *parent link) but can you just comment it out with
// rem: it leask
added?
also, I thought Stefan was chasing earlier leak, may be independent from this one
On Mon, Jan 23, 2023 at 8:31 PM Andrew Randrianasulu < [email protected]> wrote:
вт, 24 янв. 2023 г., 06:28 Phyllis Smith <[email protected]>:
Andrew, I did get a little confused by part 1 and part 2 of that checkin. Did I get it wrong?
no ... I hope.
does removing line I introduced
total_lock->unlock();
help with leak?
On Mon, Jan 23, 2023 at 8:26 PM Phyllis Smith <[email protected]> wrote:
Andrew, I found where the problem was introduced. Dec. 6, 2022 GIT is good. 3c7c8a08800c3e100388996f0e2c2eea9761ebe1 Dec.27, 2022 GIT is bad. 175a7314e8e927128787feeb7ba5f42530f0a319
On Mon, Jan 23, 2023 at 7:51 PM Andrew Randrianasulu < [email protected]> wrote:
> > > вт, 24 янв. 2023 г., 02:44 Stefan de Konink <[email protected]>: > >> Hi Andrew, >> >> When changing this, to be more in line with the "HV" code... >> >> diff --git a/cinelerra-5.1/cinelerra/playbackengine.C >> b/cinelerra-5.1/cinelerra/playbackengine.C >> index 815e506f..3c2a9368 100644 >> --- a/cinelerra-5.1/cinelerra/playbackengine.C >> +++ b/cinelerra-5.1/cinelerra/playbackengine.C >> @@ -172,9 +172,9 @@ void PlaybackEngine::create_cache() >> { >> cache_lock->lock("PlaybackEngine::create_cache"); >> if( audio_cache ) >> - audio_cache->remove_user(); >> + delete audio_cache; >> if( video_cache ) >> - video_cache->remove_user(); >> + delete video_cache; >> audio_cache = new CICache(preferences); >> video_cache = new CICache(preferences); >> cache_lock->unlock(); >> >> >> ...will result in a fix for the memory leak, but does cause a >> warning >> message. >> >> Garbage::~Garbage: title=CICache users=1 was not deleted by >> Garbage::remove_user >> >> >> This was changed with f5725c7e (7/4/20). > > > > https://git.cinelerra-gg.org/git/?p=goodguy/cinelerra.git;a=blobdiff;f=cinel... > > > hm .... > > may be you also can try to restore this idea about calling new only > if cache does not exist ... > > @@ -162,10 +164,12 @@ void PlaybackEngine::wait_render_engine() > > void PlaybackEngine::create_cache() > { > - if(audio_cache) { delete audio_cache; audio_cache = 0; } > - if(video_cache) { delete video_cache; video_cache = 0; } > - if(!audio_cache) audio_cache = new CICache(preferences); > - if(!video_cache) video_cache = new CICache(preferences); > + if( audio_cache ) > + audio_cache->remove_user(); > + if( video_cache ) > + video_cache->remove_user(); > + audio_cache = new CICache(preferences); > + video_cache = new CICache(preferences); > } > > > > > But also shows that many unrelated >> changes cause real issues to hide. I found this by hand, not even >> using git >> bisect. I understand the urge for the garbage collector, but for >> some >> reason not behaving. What I notice is that the 'last' object is >> empty when >> it is actually destroyed by the garbage collector (~CICache). What >> is the >> best way going forward? >> >> -- >> Stefan >> >
On Tuesday, January 24, 2023 5:11:42 AM CET, Phyllis Smith wrote:
Better to have EDL undo problem then run out of memory.
After a night of sleep I think what should happen is not "create" the cache if it already exists. For me it does not make sense to have "multiple caches". Now the remaining question is, would the cache get cleaned up automatically without running out of memory? I am going to try some things today. -- Stefan
On Tuesday, January 24, 2023 4:26:24 AM CET, Phyllis Smith wrote:
Andrew, I found where the problem was introduced. Dec. 6, 2022 GIT is good. 3c7c8a08800c3e100388996f0e2c2eea9761ebe1 Dec.27, 2022 GIT is bad. 175a7314e8e927128787feeb7ba5f42530f0a319
Please be aware that there is a commit in between also touching the cache on the same place (which in this case does not leak for me, but does cause a very high cpu usage). 938dfbb92e41044bee37394ba72af83c61d7cd87 When removing the 2 EDL-lines, this resolves the problem for me as well. diff --git a/cinelerra-5.1/cinelerra/cache.C b/cinelerra-5.1/cinelerra/cache.C index 9186f1f0..0525e38d 100644 --- a/cinelerra-5.1/cinelerra/cache.C +++ b/cinelerra-5.1/cinelerra/cache.C @@ -123,8 +123,8 @@ File* CICache::check_out(Asset *asset, EDL *edl, int block) } //printf("users: %i \n", users ); - EDL *parent = edl->parent_edl; - if(parent) + // EDL *parent = edl->parent_edl; + // if(parent) remove_user(); total_lock->unlock(); //printf("check out %p %lx %s\n", current, tid, asset->path); And I actually wonder if it was intended as if(!parent) - for me this also does not make sense - but I first need to understand what Andrew was trying to solve. You mentioned EDL undo... diff --git a/cinelerra-5.1/cinelerra/cache.C b/cinelerra-5.1/cinelerra/cache.C index 9186f1f0..35d4cd06 100644 --- a/cinelerra-5.1/cinelerra/cache.C +++ b/cinelerra-5.1/cinelerra/cache.C @@ -124,8 +124,9 @@ File* CICache::check_out(Asset *asset, EDL *edl, int block) //printf("users: %i \n", users ); EDL *parent = edl->parent_edl; - if(parent) - remove_user(); + if(!parent) { + remove_user(); + } total_lock->unlock(); //printf("check out %p %lx %s\n", current, tid, asset->path); return current ? current->file : 0; -- Stefan
вт, 24 янв. 2023 г., 13:35 Stefan de Konink <[email protected]>:
On Tuesday, January 24, 2023 4:26:24 AM CET, Phyllis Smith wrote:
Andrew, I found where the problem was introduced. Dec. 6, 2022 GIT is good. 3c7c8a08800c3e100388996f0e2c2eea9761ebe1 Dec.27, 2022 GIT is bad. 175a7314e8e927128787feeb7ba5f42530f0a319
Please be aware that there is a commit in between also touching the cache on the same place (which in this case does not leak for me, but does cause a very high cpu usage).
yeah, it was two-part patch I botched, it seems.
938dfbb92e41044bee37394ba72af83c61d7cd87
When removing the 2 EDL-lines, this resolves the problem for me as well.
diff --git a/cinelerra-5.1/cinelerra/cache.C b/cinelerra-5.1/cinelerra/cache.C index 9186f1f0..0525e38d 100644 --- a/cinelerra-5.1/cinelerra/cache.C +++ b/cinelerra-5.1/cinelerra/cache.C @@ -123,8 +123,8 @@ File* CICache::check_out(Asset *asset, EDL *edl, int block) }
//printf("users: %i \n", users ); - EDL *parent = edl->parent_edl; - if(parent) + // EDL *parent = edl->parent_edl; + // if(parent) remove_user(); total_lock->unlock(); //printf("check out %p %lx %s\n", current, tid, asset->path);
And I actually wonder if it was intended as if(!parent) - for me this also does not make sense - but I first need to understand what Andrew was trying to solve. You mentioned EDL undo...
not just edl undo, but specifically undo-just-right-after loading nested edl - probably peculiarity of my system, because it does not show on Phyllis systems :| I think just reverting most of it, leaving only commented printf for further investigating on my end is right course of action. Sorry!
diff --git a/cinelerra-5.1/cinelerra/cache.C b/cinelerra-5.1/cinelerra/cache.C index 9186f1f0..35d4cd06 100644 --- a/cinelerra-5.1/cinelerra/cache.C +++ b/cinelerra-5.1/cinelerra/cache.C @@ -124,8 +124,9 @@ File* CICache::check_out(Asset *asset, EDL *edl, int block)
//printf("users: %i \n", users ); EDL *parent = edl->parent_edl; - if(parent) - remove_user(); + if(!parent) { + remove_user(); + } total_lock->unlock(); //printf("check out %p %lx %s\n", current, tid, asset->path); return current ? current->file : 0;
-- Stefan
On Tuesday, January 24, 2023 11:39:45 AM CET, Andrew Randrianasulu wrote:
not just edl undo, but specifically undo-just-right-after loading nested edl - probably peculiarity of my system, because it does not show on Phyllis systems :|
I think just reverting most of it, leaving only commented printf for further investigating on my end is right course of action.
In my perspective the 'total_lock->unlock()' does make sense, because would not be unlocked otherwise. If you can record if we can still reproduce the original issue, that would be helpful. -- Stefan
вт, 24 янв. 2023 г., 13:48 Stefan de Konink <[email protected]>:
On Tuesday, January 24, 2023 11:39:45 AM CET, Andrew Randrianasulu wrote:
not just edl undo, but specifically undo-just-right-after loading nested edl - probably peculiarity of my system, because it does not show on Phyllis systems :|
I think just reverting most of it, leaving only commented printf for further investigating on my end is right course of action.
In my perspective the 'total_lock->unlock()' does make sense, because would not be unlocked otherwise.
If you can record if we can still reproduce the original issue, that would be helpful.
hm, my current build from 29 dec 2022 already had "fix", so I think simplest way of action is to revert/comment it out on git master, I pull and rebuild and post gdb backtrace (I tried to replicate it on termux install, but run into termux-specific audio crash ... tried to attack it blindly, predictably no positive results)
-- Stefan
On Tuesday, January 24, 2023 11:54:26 AM CET, Andrew Randrianasulu wrote:
hm, my current build from 29 dec 2022 already had "fix", so I think simplest way of action is to revert/comment it out on git master, I pull and rebuild and post gdb backtrace (I tried to replicate it on termux install, but run into termux-specific audio crash ... tried to attack it blindly, predictably no positive results)
You can just comment out the two lines of code in your local repository right? -- Stefan
вт, 24 янв. 2023 г., 13:57 Stefan de Konink <[email protected]>:
On Tuesday, January 24, 2023 11:54:26 AM CET, Andrew Randrianasulu wrote:
hm, my current build from 29 dec 2022 already had "fix", so I think simplest way of action is to revert/comment it out on git master, I pull and rebuild and post gdb backtrace (I tried to replicate it on termux install, but run into termux-specific audio crash ... tried to attack it blindly, predictably no positive results)
You can just comment out the two lines of code in your local repository right?
actually i just commented if condition ..... yeah, build started. Just I tend to spend time productively scrolling https://www.antipope.org/charlie/blog-static/2023/01/make-up-a-guy.html#comm... current position: comment number 378 :-)
My crash - you probably can try to load normal video via recently-loaded and then load nested xml on new tracks, and undo this load. for me confusion come from negative tids, but I guess it sort of expected? (there is difference between kernel and glibc, but I not dared to change this) guess real check should be ...somewhere else up in call chain. вт, 24 янв. 2023 г., 14:04 Andrew Randrianasulu <[email protected]>:
вт, 24 янв. 2023 г., 13:57 Stefan de Konink <[email protected]>:
On Tuesday, January 24, 2023 11:54:26 AM CET, Andrew Randrianasulu wrote:
hm, my current build from 29 dec 2022 already had "fix", so I think simplest way of action is to revert/comment it out on git master, I pull and rebuild and post gdb backtrace (I tried to replicate it on termux install, but run into termux-specific audio crash ... tried to attack it blindly, predictably no positive results)
You can just comment out the two lines of code in your local repository right?
actually i just commented if condition .....
yeah, build started. Just I tend to spend time productively scrolling
https://www.antipope.org/charlie/blog-static/2023/01/make-up-a-guy.html#comm...
current position: comment number 378
:-)
participants (3)
-
Andrew Randrianasulu -
Phyllis Smith -
Stefan de Konink