From 55f30b6a58b887682d81ea99fe3432614c16fa0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hugo=20Beauz=C3=A9e-Luyssen?= Date: Wed, 26 Jan 2022 15:56:15 +0100 Subject: [PATCH 1/3] parser: Ensure parser doesn't swallow new tasks notification --- src/parser/Parser.cpp | 12 +++++++++++- src/parser/Parser.h | 1 + 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/parser/Parser.cpp b/src/parser/Parser.cpp index f408dba7..4142200f 100644 --- a/src/parser/Parser.cpp +++ b/src/parser/Parser.cpp @@ -45,6 +45,7 @@ Parser::Parser( MediaLibrary* ml, FsHolder* fsHolder ) , m_callback( nullptr ) , m_opScheduled( 0 ) , m_opDone( 0 ) + , m_completionSignaled( false ) { } @@ -197,11 +198,20 @@ void Parser::updateStats() if ( m_callback == nullptr ) return; assert( m_opScheduled >= m_opDone ); - if ( m_opDone % 10 == 0 || m_opScheduled == m_opDone ) + /* + * We don't want to spam the callback receiver each time we're done parsing + * an item, however we must signal progress when: + * - All tasks have been processed + * - We signaled that all tasks were processed before, and we have new tasks + * to process now. + */ + if ( m_opDone % 10 == 0 || m_opScheduled == m_opDone || + m_completionSignaled == true ) { LOG_DEBUG( "Updating progress: operations scheduled ", m_opScheduled, "; operations done: ", m_opDone ); m_callback->onParsingStatsUpdated( m_opDone, m_opScheduled ); + m_completionSignaled = m_opScheduled == m_opDone; } } diff --git a/src/parser/Parser.h b/src/parser/Parser.h index 8139f0fe..3dc55a0f 100644 --- a/src/parser/Parser.h +++ b/src/parser/Parser.h @@ -104,6 +104,7 @@ private: IMediaLibraryCb* m_callback; uint32_t m_opScheduled; uint32_t m_opDone; + bool m_completionSignaled; }; } -- GitLab From 66adec4023e53f92be831f85bc0c6a49746659a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hugo=20Beauz=C3=A9e-Luyssen?= Date: Wed, 26 Jan 2022 16:00:08 +0100 Subject: [PATCH 2/3] tests: samples: Let meson interrupt the tests itself In the linux configuration, we display all threads stacktraces on timeout, which is more valuable than just knowing that it timedout. This is also more convenient to attach a debugger without rushing when the tests timeout locally --- test/samples/Tester.cpp | 17 ++++++++--------- test/samples/Tester.h | 10 +++++----- test/samples/main.cpp | 33 +++++++++++++++------------------ 3 files changed, 28 insertions(+), 32 deletions(-) diff --git a/test/samples/Tester.cpp b/test/samples/Tester.cpp index 99f9b1fb..49503dbe 100644 --- a/test/samples/Tester.cpp +++ b/test/samples/Tester.cpp @@ -56,11 +56,11 @@ MockCallback::MockCallback() { } -bool MockCallback::waitForParsingComplete() +void MockCallback::waitForParsingComplete() { std::unique_lock lock{ m_parsingMutex }; // Wait for a while, generating snapshots can be heavy... - return m_parsingCompleteVar.wait_for( lock, std::chrono::seconds{ 20 }, [this]() { + m_parsingCompleteVar.wait( lock, [this]() { return m_parserDone && m_discoveryCompleted; }); } @@ -160,22 +160,21 @@ void MockResumeCallback::reinit() m_parserDone = false; } -bool MockResumeCallback::waitForDiscoveryComplete() +void MockResumeCallback::waitForDiscoveryComplete() { std::unique_lock lock{ m_parsingMutex }; - return m_discoveryCompletedVar.wait_for( lock, std::chrono::seconds{ 20 }, [this]() { + m_discoveryCompletedVar.wait( lock, [this]() { return m_discoveryCompleted; }); } -bool MockResumeCallback::waitForParsingComplete() +void MockResumeCallback::waitForParsingComplete() { std::unique_lock lock{ m_parsingMutex }; // Reimplement without checking for discovery complete. This class is meant to be used // in 2 steps: waiting for discovery completed, then for parsing completed assert( m_discoveryCompleted == true ); - // Wait for a while, generating snapshots can be heavy... - return m_parsingCompleteVar.wait_for( lock, std::chrono::seconds{ 20 }, [this]() { + m_parsingCompleteVar.wait( lock, [this]() { return m_parserDone; }); } @@ -917,11 +916,11 @@ void MockCallback::prepareForPlaylistReload() m_parserDone = false; } -bool MockCallback::waitForPlaylistReload() +void MockCallback::waitForPlaylistReload() { std::unique_lock lock{ m_parsingMutex }; // Wait for a while, generating snapshots can be heavy... - return m_parsingCompleteVar.wait_for( lock, std::chrono::seconds{ 20 }, [this]() { + m_parsingCompleteVar.wait( lock, [this]() { return m_parserDone; }); } diff --git a/test/samples/Tester.h b/test/samples/Tester.h index c6b9d7c0..de7f65e4 100644 --- a/test/samples/Tester.h +++ b/test/samples/Tester.h @@ -49,14 +49,14 @@ class MockCallback : public mock::NoopCallback { public: MockCallback(); - virtual bool waitForParsingComplete(); - virtual bool waitForDiscoveryComplete() { return true; } + virtual void waitForParsingComplete(); + virtual void waitForDiscoveryComplete() {} virtual bool waitForRemovalComplete(); virtual void reinit(); void prepareWaitForThumbnail( MediaPtr media ); bool waitForThumbnail(); void prepareForPlaylistReload(); - bool waitForPlaylistReload(); + void waitForPlaylistReload(); void prepareForDiscovery( uint32_t nbEntryPointsExpected ); void prepareForRemoval( uint32_t nbEntryPointsRemovalExpected ); @@ -84,8 +84,8 @@ protected: class MockResumeCallback : public MockCallback { public: - virtual bool waitForDiscoveryComplete() override; - virtual bool waitForParsingComplete() override; + virtual void waitForDiscoveryComplete() override; + virtual void waitForParsingComplete() override; virtual void onDiscoveryCompleted() override; virtual void reinit() override; diff --git a/test/samples/main.cpp b/test/samples/main.cpp index db5edcf6..2c5e7769 100644 --- a/test/samples/main.cpp +++ b/test/samples/main.cpp @@ -37,14 +37,14 @@ static void Parse( Tests* T ) { - ASSERT_TRUE( T->m_cb->waitForParsingComplete() ); + T->m_cb->waitForParsingComplete(); T->runChecks(); } static void ParseTwice( Tests* T ) { - ASSERT_TRUE( T->m_cb->waitForParsingComplete() ); + T->m_cb->waitForParsingComplete(); T->runChecks(); @@ -66,46 +66,46 @@ static void ParseTwice( Tests* T ) T->m_ml->discover( utils::file::toMrl( samplesDir ) ); } - ASSERT_TRUE( T->m_cb->waitForParsingComplete() ); + T->m_cb->waitForParsingComplete(); T->runChecks(); } static void RunResumeTests( ResumeTests* T ) { - ASSERT_TRUE( T->m_cb->waitForDiscoveryComplete() ); + T->m_cb->waitForDiscoveryComplete(); auto testMl = static_cast( T->m_ml.get() ); testMl->forceParserStart(); - ASSERT_TRUE( T->m_cb->waitForParsingComplete() ); + T->m_cb->waitForParsingComplete(); T->runChecks(); } static void Rescan( ResumeTests* T ) { - ASSERT_TRUE( T->m_cb->waitForDiscoveryComplete() ); + T->m_cb->waitForDiscoveryComplete(); auto testMl = static_cast( T->m_ml.get() ); testMl->forceParserStart(); - ASSERT_TRUE( T->m_cb->waitForParsingComplete() ); + T->m_cb->waitForParsingComplete(); T->m_cb->reinit(); T->m_ml->forceRescan(); - ASSERT_TRUE( T->m_cb->waitForParsingComplete() ); + T->m_cb->waitForParsingComplete(); T->runChecks(); } static void RunRefreshTests( RefreshTests* T ) { - ASSERT_TRUE( T->m_cb->waitForDiscoveryComplete() ); - ASSERT_TRUE( T->m_cb->waitForParsingComplete() ); + T->m_cb->waitForDiscoveryComplete(); + T->m_cb->waitForParsingComplete(); T->runChecks(); T->m_cb->reinit(); T->forceRefresh(); - ASSERT_TRUE( T->m_cb->waitForParsingComplete() ); + T->m_cb->waitForParsingComplete(); T->runChecks(); } @@ -116,7 +116,7 @@ static void ReplaceVlcInstance( Tests* T ) T->m_ml->setExternalLibvlcInstance( inst.get() ); /* Replacing the instance will stop the discoverer so let's resume it */ T->m_ml->reload(); - ASSERT_TRUE( T->m_cb->waitForParsingComplete() ); + T->m_cb->waitForParsingComplete(); T->runChecks(); } @@ -127,8 +127,7 @@ static void RunBackupRestorePlaylist( BackupRestorePlaylistTests* T ) ASSERT_TRUE( utils::fs::isDirectory( samplesFolder ) ); samplesFolder = utils::fs::toAbsolute( samplesFolder ); T->m_ml->discover( utils::file::toMrl( samplesFolder ) ); - auto res = T->m_cb->waitForParsingComplete(); - ASSERT_TRUE( res ); + T->m_cb->waitForParsingComplete(); // Now we should have discovered some media auto media = T->m_ml->audioFiles( nullptr )->all(); @@ -154,8 +153,7 @@ static void RunBackupRestorePlaylist( BackupRestorePlaylistTests* T ) T->m_cb->prepareForPlaylistReload(); T->m_ml->clearDatabase( true ); - res = T->m_cb->waitForPlaylistReload(); - ASSERT_TRUE( res ); + T->m_cb->waitForPlaylistReload(); auto playlists = T->m_ml->playlists( PlaylistType::All, nullptr )->all(); ASSERT_EQ( 2u, playlists.size() ); @@ -178,8 +176,7 @@ static void RunBackupRestorePlaylist( BackupRestorePlaylistTests* T ) */ T->m_cb->reinit(); T->m_ml->discover( utils::file::toMrl( samplesFolder ) ); - res = T->m_cb->waitForParsingComplete(); - ASSERT_TRUE( res ); + T->m_cb->waitForParsingComplete(); media = playlist1->media( nullptr )->all(); ASSERT_EQ( m1->title(), media[0]->title() ); ASSERT_EQ( m2->title(), media[1]->title() ); -- GitLab From c996d355b4e53bddfbcfc74e52af7cc7267f6bbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hugo=20Beauz=C3=A9e-Luyssen?= Date: Wed, 26 Jan 2022 16:29:06 +0100 Subject: [PATCH 3/3] tests: samples: Rely on idle state to signal discover/parse completion --- test/samples/Tester.cpp | 11 ++++++----- test/samples/Tester.h | 1 + 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/test/samples/Tester.cpp b/test/samples/Tester.cpp index 49503dbe..c2f70dfb 100644 --- a/test/samples/Tester.cpp +++ b/test/samples/Tester.cpp @@ -109,7 +109,6 @@ void MockCallback::onDiscoveryCompleted() { std::lock_guard lock( m_parsingMutex ); m_discoveryCompleted = true; - m_parsingCompleteVar.notify_all(); } void MockCallback::onParsingStatsUpdated( uint32_t done, uint32_t scheduled ) @@ -117,10 +116,6 @@ void MockCallback::onParsingStatsUpdated( uint32_t done, uint32_t scheduled ) std::lock_guard lock( m_parsingMutex ); m_parserDone = done == scheduled; - if ( m_parserDone == false ) - return; - - m_parsingCompleteVar.notify_all(); } void MockCallback::onMediaThumbnailReady( MediaPtr media, ThumbnailSizeType, @@ -143,6 +138,12 @@ void MockCallback::onEntryPointRemoved( const std::string& entryPoint, bool ) if ( --m_nbEntryPointsRemovalExpected > 0 ) return; m_removalCompleted = true; +} + +void MockCallback::onBackgroundTasksIdleChanged( bool idle ) +{ + if ( idle == false ) + return; m_parsingCompleteVar.notify_all(); } diff --git a/test/samples/Tester.h b/test/samples/Tester.h index de7f65e4..43c12017 100644 --- a/test/samples/Tester.h +++ b/test/samples/Tester.h @@ -67,6 +67,7 @@ protected: virtual void onMediaThumbnailReady( MediaPtr media, ThumbnailSizeType sizeType, bool success ) override; virtual void onEntryPointRemoved( const std::string& entryPoint, bool res ) override; + virtual void onBackgroundTasksIdleChanged( bool idle ) override; compat::ConditionVariable m_parsingCompleteVar; compat::Mutex m_parsingMutex; -- GitLab