From 6cd71d99d25ddf986b8db4913d3e9e56546115c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Liz=C3=A9?= Date: Wed, 6 Feb 2019 14:15:25 +0000 Subject: [PATCH] blink/bindings: Park ParkableStrings in foreground as well. This CL adds an "aging" mechanism to parkable strings, and parks "old" strings, regardless of whether the renderer is in foreground or background. The "aging" terminology and mechanism are similar to the swapping logic in the linux kernel, that is: - A string becomes young when it's accessed - Periodically, young strings become old - Old strings are parked Here, the periodic tick only happens as long as progress is made, ensuring that an idle renderer stays idle, and a string can only become old if there are no "users", that is it is neither locked nor referenced externally. From local testing on Linux x86_64, this reduces memory by ~15MB for an active (logged-out) facebook tab, which is ~10% of Private Memory Footprint on the same tab. Total main thread CPU overhead for ~2min of active use of the page is <50ms. See the linked bug for more details and results. This new behavior is guarded by a feature flag which is disabled by default. Bug: 924164 Change-Id: I6ce59c74fd2608b4a2b543d6220e01e7a9ca1b48 Reviewed-on: https://chromium-review.googlesource.com/c/1426784 Commit-Queue: Benoit L Reviewed-by: Kentaro Hara Cr-Commit-Position: refs/heads/master@{#629564} --- .../platform/bindings/parkable_string.cc | 190 ++++++++++---- .../platform/bindings/parkable_string.h | 46 +++- .../bindings/parkable_string_manager.cc | 71 +++++- .../bindings/parkable_string_manager.h | 9 +- .../platform/bindings/parkable_string_test.cc | 240 ++++++++++++++++-- 5 files changed, 481 insertions(+), 75 deletions(-) diff --git a/third_party/blink/renderer/platform/bindings/parkable_string.cc b/third_party/blink/renderer/platform/bindings/parkable_string.cc index df13aa4ab0e583..976b5d4a565e47 100644 --- a/third_party/blink/renderer/platform/bindings/parkable_string.cc +++ b/third_party/blink/renderer/platform/bindings/parkable_string.cc @@ -151,8 +151,33 @@ struct CompressionTaskParams final { // // See |Park()| for (1), |OnParkingCompleteOnMainThread()| for 2-3, and // |Unpark()| for (4). +// +// Each state can be combined with a string that is either old or +// young. Examples below: +// - kUnParked: +// - Old: old strings are not necessarily parked +// - Young: a string starts young and unparked. +// - kParkingInProgress: +// - Old: The string wasn't touched since parking started +// - Young: The string was either touched or locked since parking started +// - kParked: +// - Old: Parked, and not touched nor locked since then +// - Young: Lock() makes a string young but doesn't unpark it. enum class ParkableStringImpl::State { kUnparked, kParkingInProgress, kParked }; +// Current "ownership" status of the underlying data. +// +// - kUnreferencedExternally: |string_| is not referenced externally, and the +// class is free to change it. +// - kTooManyReferences: |string_| has multiple references pointing to it, +// cannot change it. +// - kLocked: |this| is locked. +enum class ParkableStringImpl::Status { + kUnreferencedExternally, + kTooManyReferences, + kLocked +}; + ParkableStringImpl::ParkableStringImpl(scoped_refptr&& impl, ParkableState parkable) : mutex_(), @@ -160,6 +185,7 @@ ParkableStringImpl::ParkableStringImpl(scoped_refptr&& impl, state_(State::kUnparked), string_(std::move(impl)), compressed_(nullptr), + is_young_(true), may_be_parked_(parkable == ParkableState::kParkable), is_8bit_(string_.Is8Bit()), length_(string_.length()) @@ -189,8 +215,20 @@ ParkableStringImpl::~ParkableStringImpl() { void ParkableStringImpl::Lock() { MutexLocker locker(mutex_); lock_depth_ += 1; + // Make young as this is a strong (but not certain) indication that the string + // will be accessed soon. + MakeYoung(); +} + +#if defined(ADDRESS_SANITIZER) + +void ParkableStringImpl::LockWithoutMakingYoung() { + MutexLocker locker(mutex_); + lock_depth_ += 1; } +#endif // defined(ADDRESS_SANITIZER) + void ParkableStringImpl::Unlock() { MutexLocker locker(mutex_); DCHECK_GT(lock_depth_, 0); @@ -205,9 +243,10 @@ void ParkableStringImpl::Unlock() { // to use the string until the end of the current owning thread task. // Requires DCHECK_IS_ON() for the |owning_thread_| check. // - // Checking the owning thread first as CanParkNow() can only be called from - // the owning thread. - if (owning_thread_ == CurrentThread() && CanParkNow()) { + // Checking the owning thread first as |CurrentStatus()| can only be called + // from the owning thread. + if (owning_thread_ == CurrentThread() && may_be_parked() && + CurrentStatus() == Status::kUnreferencedExternally) { AsanPoisonString(string_); } #endif // defined(ADDRESS_SANITIZER) && DCHECK_IS_ON() @@ -219,9 +258,15 @@ void ParkableStringImpl::PurgeMemory() { compressed_ = nullptr; } +void ParkableStringImpl::MakeYoung() { + mutex_.AssertAcquired(); + is_young_ = true; +} + const String& ParkableStringImpl::ToString() { AssertOnValidThread(); MutexLocker locker(mutex_); + MakeYoung(); AsanUnpoisonString(string_); Unpark(); @@ -233,49 +278,101 @@ unsigned ParkableStringImpl::CharactersSizeInBytes() const { return length_ * (is_8bit() ? sizeof(LChar) : sizeof(UChar)); } -bool ParkableStringImpl::Park(ParkingMode mode) { - AssertOnValidThread(); +ParkableStringImpl::AgeOrParkResult ParkableStringImpl::MaybeAgeOrParkString() { MutexLocker locker(mutex_); - DCHECK(may_be_parked_); - if (state_ == State::kUnparked && CanParkNow()) { - // Parking can proceed synchronously. - if (has_compressed_data()) { - RecordParkingAction(ParkingAction::kParkedInBackground); - state_ = State::kParked; - ParkableStringManager::Instance().OnParked(this, string_.Impl()); - - // Must unpoison the memory before releasing it. - AsanUnpoisonString(string_); - string_ = String(); - } else if (mode == ParkingMode::kAlways) { - // |string_|'s data should not be touched except in the compression task. - AsanPoisonString(string_); - // |params| keeps |this| alive until |OnParkingCompleteOnMainThread()|. - auto params = std::make_unique( - this, string_.Bytes(), string_.CharactersSizeInBytes(), - Thread::Current()->GetTaskRunner()); - background_scheduler::PostOnBackgroundThread( - FROM_HERE, CrossThreadBind(&ParkableStringImpl::CompressInBackground, - WTF::Passed(std::move(params)))); - state_ = State::kParkingInProgress; + AssertOnValidThread(); + DCHECK(may_be_parked()); + DCHECK(!is_parked()); + + Status status = CurrentStatus(); + if (is_young()) { + if (status == Status::kUnreferencedExternally) + is_young_ = false; + } else { + if (state_ == State::kParkingInProgress) + return AgeOrParkResult::kSuccessOrTransientFailure; + + if (CanParkNow()) { + ParkInternal(ParkingMode::kAlways); + return AgeOrParkResult::kSuccessOrTransientFailure; } } - return state_ == State::kParked || state_ == State::kParkingInProgress; + // External references to a string can be long-lived, cannot provide a + // progress guarantee for this string. + return status == Status::kTooManyReferences + ? AgeOrParkResult::kNonTransientFailure + : AgeOrParkResult::kSuccessOrTransientFailure; +} + +bool ParkableStringImpl::Park(ParkingMode mode) { + MutexLocker locker(mutex_); + AssertOnValidThread(); + DCHECK(may_be_parked()); + + if (state_ == State::kParkingInProgress || state_ == State::kParked) + return true; + + // Making the string old to cancel parking if it is accessed/locked before + // parking is complete. + is_young_ = false; + if (!CanParkNow()) + return false; + + ParkInternal(mode); + return true; +} + +void ParkableStringImpl::ParkInternal(ParkingMode mode) { + mutex_.AssertAcquired(); + DCHECK_EQ(State::kUnparked, state_); + DCHECK(!is_young()); + DCHECK(CanParkNow()); + + // Parking can proceed synchronously. + if (has_compressed_data()) { + RecordParkingAction(ParkingAction::kParkedInBackground); + state_ = State::kParked; + ParkableStringManager::Instance().OnParked(this, string_.Impl()); + + // Must unpoison the memory before releasing it. + AsanUnpoisonString(string_); + string_ = String(); + } else if (mode == ParkingMode::kAlways) { + // |string_|'s data should not be touched except in the compression task. + AsanPoisonString(string_); + // |params| keeps |this| alive until |OnParkingCompleteOnMainThread()|. + auto params = std::make_unique( + this, string_.Bytes(), string_.CharactersSizeInBytes(), + Thread::Current()->GetTaskRunner()); + background_scheduler::PostOnBackgroundThread( + FROM_HERE, CrossThreadBind(&ParkableStringImpl::CompressInBackground, + WTF::Passed(std::move(params)))); + state_ = State::kParkingInProgress; + } } bool ParkableStringImpl::is_parked() const { return state_ == State::kParked; } -bool ParkableStringImpl::CanParkNow() const { +ParkableStringImpl::Status ParkableStringImpl::CurrentStatus() const { + AssertOnValidThread(); mutex_.AssertAcquired(); + DCHECK(may_be_parked()); // Can park iff: - // - the string is eligible to parking - // - There are no external reference to |string_|. Since |this| holds a - // reference to it, then we are the only one. // - |this| is not locked. - return may_be_parked_ && string_.Impl()->HasOneRef() && lock_depth_ == 0; + // - There are no external reference to |string_|. Since |this| holds a + // reference to |string_|, it must the only one. + if (lock_depth_ != 0) + return Status::kLocked; + if (!string_.Impl()->HasOneRef()) + return Status::kTooManyReferences; + return Status::kUnreferencedExternally; +} + +bool ParkableStringImpl::CanParkNow() const { + return CurrentStatus() == Status::kUnreferencedExternally && !is_young(); } void ParkableStringImpl::Unpark() { @@ -333,21 +430,23 @@ void ParkableStringImpl::OnParkingCompleteOnMainThread( std::unique_ptr> compressed) { MutexLocker locker(mutex_); DCHECK_EQ(State::kParkingInProgress, state_); + + // Always keep the compressed data. Compression is expensive, so even if the + // uncompressed representation cannot be discarded now, avoid compressing + // multiple times. This will allow synchronous parking next time. + DCHECK(!compressed_); + if (compressed) + compressed_ = std::move(compressed); + // Between |Park()| and now, things may have happened: // 1. |ToString()| or // 2. |Lock()| may have been called. // - // We only care about "surviving" calls, that is iff the string returned by - // |ToString()| is still alive, or whether we are still locked. Since this - // function is protected by the lock, no concurrent modifications can occur. - // - // Finally, since this is a distinct task from any one that can call - // |ToString()|, the invariant that the pointer stays valid until the next - // task is preserved. - if (CanParkNow() && compressed) { + // Both of these will make the string young again, and if so we don't + // discard the compressed representation yet. + if (CanParkNow() && compressed_) { RecordParkingAction(ParkingAction::kParkedInBackground); state_ = State::kParked; - compressed_ = std::move(compressed); ParkableStringManager::Instance().OnParked(this, string_.Impl()); // Must unpoison the memory before releasing it. @@ -368,7 +467,12 @@ void ParkableStringImpl::CompressInBackground( #if defined(ADDRESS_SANITIZER) // Lock the string to prevent a concurrent |Unlock()| on the main thread from // poisoning the string in the meantime. - params->string->Lock(); + // + // Don't make the string young at the same time, otherwise parking would + // always be cancelled on the main thread with address sanitizer, since the + // |OnParkingCompleteOnMainThread()| callback would be executed on a young + // string. + params->string->LockWithoutMakingYoung(); #endif // defined(ADDRESS_SANITIZER) // Compression touches the string. AsanUnpoisonString(params->string->string_); diff --git a/third_party/blink/renderer/platform/bindings/parkable_string.h b/third_party/blink/renderer/platform/bindings/parkable_string.h index 7372323abded0b..719f8aa69ab9af 100644 --- a/third_party/blink/renderer/platform/bindings/parkable_string.h +++ b/third_party/blink/renderer/platform/bindings/parkable_string.h @@ -51,6 +51,10 @@ class PLATFORM_EXPORT ParkableStringImpl final enum class ParkableState { kParkable, kNotParkable }; enum class ParkingMode { kIfCompressedDataExists, kAlways }; + enum class AgeOrParkResult { + kSuccessOrTransientFailure, + kNonTransientFailure + }; // Not all parkable strings can actually be parked. If |parkable| is // kNotParkable, then one cannot call |Park()|, and the underlying StringImpl @@ -72,12 +76,26 @@ class PLATFORM_EXPORT ParkableStringImpl final unsigned length() const { return length_; } unsigned CharactersSizeInBytes() const; + // Tries to either age or park a string: + // + // - If the string is already old, tries to park it. + // - Otherwise, tries to age it. + // + // The action doesn't necessarily succeed. either due to a temporary + // or potentially lasting condition. + // + // As parking may be synchronous, this can call back into + // ParkableStringManager. + AgeOrParkResult MaybeAgeOrParkString(); + // A parked string cannot be accessed until it has been |Unpark()|-ed. // // Parking may be synchronous, and will be if compressed data is already // available. If |mode| is |kIfCompressedDataExists|, then parking will always // be synchronous. // + // Must not be called if |may_be_parked()| returns false. + // // Returns true if the string is being parked or has been parked. bool Park(ParkingMode mode); // Returns true iff the string can be parked. This does not mean that the @@ -96,12 +114,29 @@ class PLATFORM_EXPORT ParkableStringImpl final return compressed_->size(); } + // A string can either be "young" or "old". It starts young, and transitions + // are: + // Young -> Old: By calling |MaybeAgeOrParkString()|. + // Old -> Young: When the string is accessed, either by |Lock()|-ing it or + // calling |ToString()|. + bool is_young() const { return is_young_; } + private: enum class State; - - // Whether the string can be parked now. Must be called with |mutex_| held, - // and the return value is valid as long as the mutex is held. + enum class Status; + +#if defined(ADDRESS_SANITIZER) + // See |CompressInBackground()|. Doesn't make the string young. + // May be called from any thread. + void LockWithoutMakingYoung(); +#endif // defined(ADDRESS_SANITIZER) + // May be called from any thread. Must acquire |mutex_| first. + void MakeYoung(); + // Whether the string is referenced or locked. Must be called with |mutex_| + // held, and the return value is valid as long as |mutex_| is held. + Status CurrentStatus() const; bool CanParkNow() const; + void ParkInternal(ParkingMode mode); void Unpark(); // Called on the main thread after compression is done. // |params| is the same as the one passed to |CompressInBackground()|, @@ -120,9 +155,10 @@ class PLATFORM_EXPORT ParkableStringImpl final State state_; String string_; std::unique_ptr> compressed_; + bool is_young_ : 1; - const bool may_be_parked_; - const bool is_8bit_; + const bool may_be_parked_ : 1; + const bool is_8bit_ : 1; const unsigned length_; #if DCHECK_IS_ON() diff --git a/third_party/blink/renderer/platform/bindings/parkable_string_manager.cc b/third_party/blink/renderer/platform/bindings/parkable_string_manager.cc index adf81acf0c113e..fe162fd0dceaa2 100644 --- a/third_party/blink/renderer/platform/bindings/parkable_string_manager.cc +++ b/third_party/blink/renderer/platform/bindings/parkable_string_manager.cc @@ -36,6 +36,17 @@ class OnPurgeMemoryListener : public GarbageCollected, } }; +Vector GetUnparkedStrings( + const HashMap>& + unparked_strings) { + WTF::Vector unparked; + unparked.ReserveCapacity(unparked_strings.size()); + for (ParkableStringImpl* str : unparked_strings.Values()) + unparked.push_back(str); + + return unparked; +} + } // namespace // static @@ -172,12 +183,14 @@ scoped_refptr ParkableStringManager::Add( auto new_parkable_string = base::MakeRefCounted( std::move(string), ParkableStringImpl::ParkableState::kParkable); unparked_strings_.insert(raw_ptr, new_parkable_string.get()); + ScheduleAgingTaskIfNeeded(); return new_parkable_string; } void ParkableStringManager::Remove(ParkableStringImpl* string, StringImpl* maybe_unparked_string) { DCHECK(IsMainThread()); + DCHECK(string->may_be_parked()); if (string->is_parked()) { auto it = parked_strings_.find(string); DCHECK(it != parked_strings_.end()); @@ -193,6 +206,7 @@ void ParkableStringManager::Remove(ParkableStringImpl* string, void ParkableStringManager::OnParked(ParkableStringImpl* newly_parked_string, StringImpl* previous_unparked_string) { DCHECK(IsMainThread()); + DCHECK(newly_parked_string->may_be_parked()); DCHECK(newly_parked_string->is_parked()); auto it = unparked_strings_.find(previous_unparked_string); DCHECK(it != unparked_strings_.end()); @@ -203,11 +217,13 @@ void ParkableStringManager::OnParked(ParkableStringImpl* newly_parked_string, void ParkableStringManager::OnUnparked(ParkableStringImpl* was_parked_string, StringImpl* new_unparked_string) { DCHECK(IsMainThread()); + DCHECK(was_parked_string->may_be_parked()); DCHECK(!was_parked_string->is_parked()); auto it = parked_strings_.find(was_parked_string); DCHECK(it != parked_strings_.end()); parked_strings_.erase(it); unparked_strings_.insert(new_unparked_string, was_parked_string); + ScheduleAgingTaskIfNeeded(); } void ParkableStringManager::ParkAll(ParkableStringImpl::ParkingMode mode) { @@ -227,10 +243,8 @@ void ParkableStringManager::ParkAll(ParkableStringImpl::ParkingMode mode) { // and |unparked_strings_| can contain a few 10s of strings (and we will // trigger expensive compression), or this is a subsequent one, and // |unparked_strings_| will have few entries. - WTF::Vector unparked; - unparked.ReserveCapacity(unparked_strings_.size()); - for (ParkableStringImpl* str : unparked_strings_.Values()) - unparked.push_back(str); + WTF::Vector unparked = + GetUnparkedStrings(unparked_strings_); for (ParkableStringImpl* str : unparked) { str->Park(mode); @@ -302,6 +316,53 @@ void ParkableStringManager::DropStringsWithCompressedDataAndRecordStatistics() { } } +void ParkableStringManager::AgeStringsAndPark() { + if (!base::FeatureList::IsEnabled(kCompressParkableStringsInForeground)) + return; + + has_pending_aging_task_ = false; + + WTF::Vector unparked = + GetUnparkedStrings(unparked_strings_); + + bool can_make_progress = false; + for (ParkableStringImpl* str : unparked) { + if (str->MaybeAgeOrParkString() == + ParkableStringImpl::AgeOrParkResult::kSuccessOrTransientFailure) + can_make_progress = true; + } + + // Some strings will never be parkable because there are lasting external + // references to them. Don't endlessely reschedule the aging task if we are + // not making progress (that is, no new string was either aged or parked). + // + // This ensures that the tasks will stop getting scheduled, assuming that + // the renderer is otherwise idle. Note that we cannot use "idle" tasks as + // we need to age and park strings after the renderer becomes idle, meaning + // that this has to run when the idle tasks are not. As a consequence, it + // is important to make sure that this will not reschedule tasks forever. + bool reschedule = !unparked_strings_.IsEmpty() && can_make_progress; + if (reschedule) + ScheduleAgingTaskIfNeeded(); +} + +void ParkableStringManager::ScheduleAgingTaskIfNeeded() { + if (!base::FeatureList::IsEnabled(kCompressParkableStringsInForeground)) + return; + + if (has_pending_aging_task_) + return; + + scoped_refptr task_runner = + Thread::Current()->GetTaskRunner(); + task_runner->PostDelayedTask( + FROM_HERE, + base::BindOnce(&ParkableStringManager::AgeStringsAndPark, + base::Unretained(this)), + base::TimeDelta::FromSeconds(kAgingIntervalInSeconds)); + has_pending_aging_task_ = true; +} + void ParkableStringManager::PurgeMemory() { DCHECK(IsMainThread()); DCHECK(base::FeatureList::IsEnabled(kCompressParkableStringsInBackground)); @@ -322,6 +383,7 @@ void ParkableStringManager::PurgeMemory() { void ParkableStringManager::ResetForTesting() { backgrounded_ = false; waiting_to_record_stats_ = false; + has_pending_aging_task_ = false; should_record_stats_ = false; unparked_strings_.clear(); parked_strings_.clear(); @@ -330,6 +392,7 @@ void ParkableStringManager::ResetForTesting() { ParkableStringManager::ParkableStringManager() : backgrounded_(false), waiting_to_record_stats_(false), + has_pending_aging_task_(false), should_record_stats_(false), unparked_strings_(), parked_strings_() { diff --git a/third_party/blink/renderer/platform/bindings/parkable_string_manager.h b/third_party/blink/renderer/platform/bindings/parkable_string_manager.h index 54e763184f3c64..3a68651603eada 100644 --- a/third_party/blink/renderer/platform/bindings/parkable_string_manager.h +++ b/third_party/blink/renderer/platform/bindings/parkable_string_manager.h @@ -23,6 +23,8 @@ class ParkableString; const base::Feature kCompressParkableStringsInBackground{ "CompressParkableStringsInBackground", base::FEATURE_DISABLED_BY_DEFAULT}; +const base::Feature kCompressParkableStringsInForeground{ + "CompressParkableStringsInForeground", base::FEATURE_DISABLED_BY_DEFAULT}; class PLATFORM_EXPORT ParkableStringManagerDumpProvider : public base::trace_event::MemoryDumpProvider { @@ -63,6 +65,7 @@ class PLATFORM_EXPORT ParkableStringManager { static bool ShouldPark(const StringImpl& string); // Public for testing. + constexpr static int kAgingIntervalInSeconds = 2; constexpr static int kParkingDelayInSeconds = 10; constexpr static int kStatisticsRecordingDelayInSeconds = 30; @@ -79,18 +82,22 @@ class PLATFORM_EXPORT ParkableStringManager { void ParkAll(ParkableStringImpl::ParkingMode mode); void ParkAllIfRendererBackgrounded(ParkableStringImpl::ParkingMode mode); void DropStringsWithCompressedDataAndRecordStatistics(); + void AgeStringsAndPark(); + void ScheduleAgingTaskIfNeeded(); + void ResetForTesting(); ParkableStringManager(); bool backgrounded_; bool waiting_to_record_stats_; + bool has_pending_aging_task_; bool should_record_stats_; HashMap> unparked_strings_; HashSet> parked_strings_; - friend class ParkableStringTest; + friend class ParkableStringTestBase; FRIEND_TEST_ALL_PREFIXES(ParkableStringTest, SynchronousCompression); DISALLOW_COPY_AND_ASSIGN(ParkableStringManager); }; diff --git a/third_party/blink/renderer/platform/bindings/parkable_string_test.cc b/third_party/blink/renderer/platform/bindings/parkable_string_test.cc index 3b278fcbb2b4be..5969c974f584b0 100644 --- a/third_party/blink/renderer/platform/bindings/parkable_string_test.cc +++ b/third_party/blink/renderer/platform/bindings/parkable_string_test.cc @@ -36,9 +36,9 @@ String MakeLargeString() { } // namespace -class ParkableStringTest : public ::testing::Test { +class ParkableStringTestBase : public ::testing::Test { public: - ParkableStringTest() + ParkableStringTestBase() : ::testing::Test(), scoped_task_environment_( base::test::ScopedTaskEnvironment::MainThreadType::MOCK_TIME), @@ -47,6 +47,32 @@ class ParkableStringTest : public ::testing::Test { protected: void RunPostedTasks() { scoped_task_environment_.RunUntilIdle(); } + bool ParkAndWait(const ParkableString& string) { + bool success = + string.Impl()->Park(ParkableStringImpl::ParkingMode::kAlways); + RunPostedTasks(); + return success; + } + + void SetUp() override { ParkableStringManager::Instance().ResetForTesting(); } + + void TearDown() override { + // No leaks. + CHECK_EQ(0u, ParkableStringManager::Instance().Size()); + // Delayed tasks may remain, clear the queues. + scoped_task_environment_.FastForwardUntilNoTasksRemain(); + RunPostedTasks(); + } + + base::test::ScopedTaskEnvironment scoped_task_environment_; + base::test::ScopedFeatureList scoped_feature_list_; +}; + +class ParkableStringTest : public ParkableStringTestBase { + public: + ParkableStringTest() : ParkableStringTestBase() {} + + protected: void WaitForDelayedParking() { scoped_task_environment_.FastForwardBy(base::TimeDelta::FromSeconds( ParkableStringManager::kParkingDelayInSeconds)); @@ -59,13 +85,6 @@ class ParkableStringTest : public ::testing::Test { RunPostedTasks(); } - bool ParkAndWait(const ParkableString& string) { - bool return_value = - string.Impl()->Park(ParkableStringImpl::ParkingMode::kAlways); - RunPostedTasks(); - return return_value; - } - ParkableString CreateAndParkAll() { auto& manager = ParkableStringManager::Instance(); // Checking that there are no other strings, to make sure this doesn't @@ -80,21 +99,10 @@ class ParkableStringTest : public ::testing::Test { } void SetUp() override { - ParkableStringManager::Instance().ResetForTesting(); + ParkableStringTestBase::SetUp(); scoped_feature_list_.InitAndEnableFeature( kCompressParkableStringsInBackground); } - - void TearDown() override { - // No leaks. - CHECK_EQ(0u, ParkableStringManager::Instance().Size()); - // Delayed tasks may remain, clear the queues. - scoped_task_environment_.FastForwardUntilNoTasksRemain(); - RunPostedTasks(); - } - - base::test::ScopedTaskEnvironment scoped_task_environment_; - base::test::ScopedFeatureList scoped_feature_list_; }; // The main aim of this test is to check that the compressed size of a string @@ -182,6 +190,9 @@ TEST_F(ParkableStringTest, Park) { ParkableString parkable(MakeLargeString().ReleaseImpl()); EXPECT_TRUE(parkable.may_be_parked()); EXPECT_FALSE(parkable.Impl()->is_parked()); + EXPECT_TRUE( + parkable.Impl()->Park(ParkableStringImpl::ParkingMode::kAlways)); + // Should not crash, it is allowed to call |Park()| twice in a row. EXPECT_TRUE( parkable.Impl()->Park(ParkableStringImpl::ParkingMode::kAlways)); parkable = ParkableString(); // release the reference. @@ -222,14 +233,22 @@ TEST_F(ParkableStringTest, AbortParking) { { ParkableString parkable(MakeLargeString().ReleaseImpl()); - // Transient |Lock()| or |ToString()| doesn't cancel parking. + // Transient |Lock()| or |ToString()| cancel parking. EXPECT_TRUE( parkable.Impl()->Park(ParkableStringImpl::ParkingMode::kAlways)); parkable.Impl()->Lock(); parkable.Impl()->ToString(); parkable.Impl()->Unlock(); RunPostedTasks(); + EXPECT_FALSE(parkable.Impl()->is_parked()); + + // In order to test synchronous parking below, need to park the string + // first. + EXPECT_TRUE( + parkable.Impl()->Park(ParkableStringImpl::ParkingMode::kAlways)); + RunPostedTasks(); EXPECT_TRUE(parkable.Impl()->is_parked()); + parkable.ToString(); // Synchronous parking respects locking and external references. parkable.ToString(); @@ -250,6 +269,23 @@ TEST_F(ParkableStringTest, AbortParking) { } } +TEST_F(ParkableStringTest, AbortedParkingRetainsCompressedData) { + ParkableString parkable(MakeLargeString().ReleaseImpl()); + EXPECT_TRUE(parkable.may_be_parked()); + EXPECT_FALSE(parkable.Impl()->is_parked()); + + EXPECT_TRUE(parkable.Impl()->Park(ParkableStringImpl::ParkingMode::kAlways)); + parkable.ToString(); // Cancels parking. + RunPostedTasks(); + EXPECT_FALSE(parkable.Impl()->is_parked()); + // Compressed data is not discarded. + EXPECT_TRUE(parkable.Impl()->has_compressed_data()); + + // Synchronous parking. + EXPECT_TRUE(parkable.Impl()->Park(ParkableStringImpl::ParkingMode::kAlways)); + EXPECT_TRUE(parkable.Impl()->is_parked()); +} + TEST_F(ParkableStringTest, Unpark) { base::HistogramTester histogram_tester; @@ -671,4 +707,164 @@ TEST_F(ParkableStringTest, ReportMemoryDump) { EXPECT_THAT(dump->entries(), Contains(Eq(ByRef(metadata)))); } +class ParkableStringForegroundParkingTest : public ParkableStringTestBase { + public: + ParkableStringForegroundParkingTest() : ParkableStringTestBase() {} + + protected: + void WaitForAging() { + ASSERT_TRUE(scoped_task_environment_.MainThreadHasPendingTask()); + scoped_task_environment_.FastForwardBy(base::TimeDelta::FromSeconds( + ParkableStringManager::kAgingIntervalInSeconds)); + RunPostedTasks(); + } + + void SetUp() override { + ParkableStringTestBase::SetUp(); + scoped_feature_list_.InitAndEnableFeature( + kCompressParkableStringsInForeground); + } + + void TearDown() override { + while (scoped_task_environment_.MainThreadHasPendingTask()) + WaitForAging(); + + ParkableStringTestBase::TearDown(); + } +}; + +TEST_F(ParkableStringForegroundParkingTest, Aging) { + ParkableString parkable(MakeLargeString().ReleaseImpl()); + EXPECT_TRUE(parkable.Impl()->is_young()); + WaitForAging(); + EXPECT_FALSE(parkable.Impl()->is_young()); + + parkable.Lock(); + EXPECT_TRUE(parkable.Impl()->is_young()); + // Locked strings don't age. + WaitForAging(); + EXPECT_TRUE(parkable.Impl()->is_young()); + parkable.Unlock(); + WaitForAging(); + EXPECT_FALSE(parkable.Impl()->is_young()); + + parkable.ToString(); + EXPECT_TRUE(parkable.Impl()->is_young()); + // No external reference, can age again. + WaitForAging(); + EXPECT_FALSE(parkable.Impl()->is_young()); + + // External references prevent a string from aging. + String retained = parkable.ToString(); + EXPECT_TRUE(parkable.Impl()->is_young()); + WaitForAging(); + EXPECT_TRUE(parkable.Impl()->is_young()); +} + +TEST_F(ParkableStringForegroundParkingTest, OldStringsAreParked) { + ParkableString parkable(MakeLargeString().ReleaseImpl()); + EXPECT_TRUE(parkable.Impl()->is_young()); + WaitForAging(); + EXPECT_FALSE(parkable.Impl()->is_young()); + WaitForAging(); + EXPECT_TRUE(parkable.Impl()->is_parked()); + + // Unparked, two aging cycles before parking. + parkable.ToString(); + EXPECT_FALSE(parkable.Impl()->is_parked()); + WaitForAging(); + EXPECT_FALSE(parkable.Impl()->is_parked()); + WaitForAging(); + EXPECT_TRUE(parkable.Impl()->is_parked()); + + // Unparked, two consecutive no-access aging cycles before parking. + parkable.ToString(); + EXPECT_FALSE(parkable.Impl()->is_parked()); + WaitForAging(); + EXPECT_FALSE(parkable.Impl()->is_parked()); + parkable.ToString(); + WaitForAging(); + EXPECT_FALSE(parkable.Impl()->is_parked()); +} + +TEST_F(ParkableStringForegroundParkingTest, AgingTicksStopsAndRestarts) { + ParkableString parkable(MakeLargeString().ReleaseImpl()); + EXPECT_TRUE(scoped_task_environment_.MainThreadHasPendingTask()); + WaitForAging(); + EXPECT_TRUE(scoped_task_environment_.MainThreadHasPendingTask()); + WaitForAging(); + // Nothing more to do, the tick is not re-scheduled. + WaitForAging(); + EXPECT_FALSE(scoped_task_environment_.MainThreadHasPendingTask()); + + // Unparking, the tick restarts. + parkable.ToString(); + EXPECT_TRUE(scoped_task_environment_.MainThreadHasPendingTask()); + WaitForAging(); + WaitForAging(); + // And stops again. 2 ticks to park the string (age, then park), and one + // checking that there is nothing left to do. + EXPECT_FALSE(scoped_task_environment_.MainThreadHasPendingTask()); + + // New string, restarting the tick, temporarily. + ParkableString parkable2(MakeLargeString().ReleaseImpl()); + WaitForAging(); + WaitForAging(); + WaitForAging(); + EXPECT_FALSE(scoped_task_environment_.MainThreadHasPendingTask()); +} + +TEST_F(ParkableStringForegroundParkingTest, AgingTicksStopsWithNoProgress) { + ParkableString parkable(MakeLargeString().ReleaseImpl()); + String retained = parkable.ToString(); + + EXPECT_TRUE(scoped_task_environment_.MainThreadHasPendingTask()); + WaitForAging(); + // The only string is referenced externally, nothing aging can change. + EXPECT_FALSE(scoped_task_environment_.MainThreadHasPendingTask()); + + ParkableString parkable2(MakeLargeString().ReleaseImpl()); + WaitForAging(); + EXPECT_TRUE(scoped_task_environment_.MainThreadHasPendingTask()); + WaitForAging(); + EXPECT_TRUE(scoped_task_environment_.MainThreadHasPendingTask()); + EXPECT_TRUE(parkable2.Impl()->is_parked()); + EXPECT_TRUE(scoped_task_environment_.MainThreadHasPendingTask()); + WaitForAging(); + // Once |parkable2| has been parked, back to the case where the only + // remaining strings are referenced externally. + EXPECT_FALSE(scoped_task_environment_.MainThreadHasPendingTask()); +} + +TEST_F(ParkableStringForegroundParkingTest, OnlyOneAgingTask) { + ParkableString parkable1(MakeLargeString().ReleaseImpl()); + ParkableString parkable2(MakeLargeString().ReleaseImpl()); + + // Park both, and wait for the tick to stop. + WaitForAging(); + WaitForAging(); + EXPECT_TRUE(parkable1.Impl()->is_parked()); + EXPECT_TRUE(parkable2.Impl()->is_parked()); + WaitForAging(); + EXPECT_FALSE(scoped_task_environment_.MainThreadHasPendingTask()); + + parkable1.ToString(); + parkable2.ToString(); + EXPECT_TRUE(scoped_task_environment_.MainThreadHasPendingTask()); + EXPECT_EQ(1u, scoped_task_environment_.GetPendingMainThreadTaskCount()); +} + +TEST_F(ParkableStringForegroundParkingTest, AgingParkingInProgress) { + ParkableString parkable(MakeLargeString().ReleaseImpl()); + + WaitForAging(); + parkable.Impl()->Park(ParkableStringImpl::ParkingMode::kAlways); + // Aging should work if the string is being parked. + WaitForAging(); + // The aging task is rescheduled. + EXPECT_EQ(1u, scoped_task_environment_.GetPendingMainThreadTaskCount()); + + EXPECT_TRUE(parkable.Impl()->is_parked()); +} + } // namespace blink