commit b8e2b3b04fd931a2ba0db57510b76a39ab030e2e
parent b97136c310eac98ee162bcdc1de1c446368bfaca
Author: James Teow <jteow@mozilla.com>
Date: Tue, 11 Nov 2025 02:37:13 +0000
Bug 1986285 - Part 2: Remove frecency decaying - r=mak,places-reviewers
Differential Revision: https://phabricator.services.mozilla.com/D263900
Diffstat:
11 files changed, 2 insertions(+), 201 deletions(-)
diff --git a/toolkit/components/places/Database.cpp b/toolkit/components/places/Database.cpp
@@ -1728,8 +1728,6 @@ nsresult Database::InitFunctions(mozIStorageConnection* aMainConn) {
NS_ENSURE_SUCCESS(rv, rv);
rv = StripPrefixAndUserinfoFunction::create(aMainConn);
NS_ENSURE_SUCCESS(rv, rv);
- rv = IsFrecencyDecayingFunction::create(aMainConn);
- NS_ENSURE_SUCCESS(rv, rv);
rv = NoteSyncChangeFunction::create(aMainConn);
NS_ENSURE_SUCCESS(rv, rv);
rv = InvalidateDaysOfHistoryFunction::create(aMainConn);
diff --git a/toolkit/components/places/PlacesFrecencyRecalculator.sys.mjs b/toolkit/components/places/PlacesFrecencyRecalculator.sys.mjs
@@ -390,26 +390,16 @@ export class PlacesFrecencyRecalculator {
}
/**
- * Decays frecency and adaptive history.
+ * Decays adaptive history.
*
* @returns {Promise<void>} once the process is complete. Never rejects.
*/
async decay() {
- lazy.logger.trace("Decay frecency");
+ lazy.logger.trace("Decay adaptive history.");
let timerId = Glean.places.idleFrecencyDecayTime.start();
- // Ensure moz_places_afterupdate_frecency_trigger ignores decaying
- // frecency changes.
- lazy.PlacesUtils.history.isFrecencyDecaying = true;
try {
let db = await lazy.PlacesUtils.promiseUnsafeWritableDBConnection();
await db.executeTransaction(async function () {
- // Decay all frecency rankings to reduce value of pages that haven't
- // been visited in a while.
- await db.executeCached(
- `UPDATE moz_places SET frecency = ROUND(frecency * :decay_rate)
- WHERE frecency > 0 AND recalc_frecency = 0`,
- { decay_rate: lazy.frecencyDecayRate }
- );
// Decay potentially unused adaptive entries (e.g. those that are at 1)
// to allow better chances for new entries that will start at 1.
await db.executeCached(
@@ -428,14 +418,11 @@ export class PlacesFrecencyRecalculator {
);
Glean.places.idleFrecencyDecayTime.stopAndAccumulate(timerId);
- PlacesObservers.notifyListeners([new PlacesRanking()]);
});
} catch (ex) {
Glean.places.idleFrecencyDecayTime.cancel(timerId);
console.error(ex);
lazy.logger.error(ex);
- } finally {
- lazy.PlacesUtils.history.isFrecencyDecaying = false;
}
}
diff --git a/toolkit/components/places/SQLFunctions.cpp b/toolkit/components/places/SQLFunctions.cpp
@@ -1482,38 +1482,6 @@ StripPrefixAndUserinfoFunction::OnFunctionCall(mozIStorageValueArray* aArgs,
}
////////////////////////////////////////////////////////////////////////////////
-//// Is frecency decaying function
-
-/* static */
-nsresult IsFrecencyDecayingFunction::create(mozIStorageConnection* aDBConn) {
- RefPtr<IsFrecencyDecayingFunction> function =
- new IsFrecencyDecayingFunction();
- nsresult rv = aDBConn->CreateFunction("is_frecency_decaying"_ns, 0, function);
- NS_ENSURE_SUCCESS(rv, rv);
-
- return NS_OK;
-}
-
-NS_IMPL_ISUPPORTS(IsFrecencyDecayingFunction, mozIStorageFunction)
-
-NS_IMETHODIMP
-IsFrecencyDecayingFunction::OnFunctionCall(mozIStorageValueArray* aArgs,
- nsIVariant** _result) {
- MOZ_ASSERT(aArgs);
-
-#ifdef DEBUG
- uint32_t numArgs;
- MOZ_ASSERT(NS_SUCCEEDED(aArgs->GetNumEntries(&numArgs)) && numArgs == 0);
-#endif
-
- RefPtr<nsVariant> result = new nsVariant();
- nsresult rv = result->SetAsBool(nsNavHistory::sIsFrecencyDecaying);
- NS_ENSURE_SUCCESS(rv, rv);
- result.forget(_result);
- return NS_OK;
-}
-
-////////////////////////////////////////////////////////////////////////////////
//// Should start frecency recalculation function
/* static */
diff --git a/toolkit/components/places/SQLFunctions.h b/toolkit/components/places/SQLFunctions.h
@@ -558,32 +558,6 @@ class StripPrefixAndUserinfoFunction final : public mozIStorageFunction {
};
////////////////////////////////////////////////////////////////////////////////
-//// Is frecency decaying function
-
-/**
- * Returns nsNavHistory::IsFrecencyDecaying().
- *
- * @return
- * True if frecency is currently decaying and false otherwise.
- */
-class IsFrecencyDecayingFunction final : public mozIStorageFunction {
- public:
- NS_DECL_THREADSAFE_ISUPPORTS
- NS_DECL_MOZISTORAGEFUNCTION
-
- /**
- * Registers the function with the specified database connection.
- *
- * @param aDBConn
- * The database connection to register with.
- */
- static nsresult create(mozIStorageConnection* aDBConn);
-
- private:
- ~IsFrecencyDecayingFunction() = default;
-};
-
-////////////////////////////////////////////////////////////////////////////////
//// Should start frecency recalculation function
/**
diff --git a/toolkit/components/places/nsINavHistoryService.idl b/toolkit/components/places/nsINavHistoryService.idl
@@ -1150,13 +1150,6 @@ interface nsINavHistoryService : nsISupports
unsigned long long hashURL(in ACString aSpec, [optional] in ACString aMode);
/**
- * Whether frecency is in the process of being decayed. The value can also
- * be read through the is_frecency_decaying() SQL function exposed by Places
- * database connections.
- */
- attribute boolean isFrecencyDecaying;
-
- /**
* Whether alternative frecency is enabled. This is preferred over directly
* checking the feature gating preference, as it ensures consistency between
* the backend and frontend by avoiding discrepancies in preference values
diff --git a/toolkit/components/places/nsNavHistory.cpp b/toolkit/components/places/nsNavHistory.cpp
@@ -506,7 +506,6 @@ mozilla::Maybe<nsCString> nsNavHistory::GetTargetFolderGuid(
Atomic<int64_t> nsNavHistory::sLastInsertedPlaceId(0);
Atomic<int64_t> nsNavHistory::sLastInsertedVisitId(0);
-Atomic<bool> nsNavHistory::sIsFrecencyDecaying(false);
Atomic<bool> nsNavHistory::sShouldStartFrecencyRecalculation(false);
void // static
@@ -1820,19 +1819,6 @@ nsNavHistory::MarkPageAsFollowedLink(nsIURI* aURI) {
}
NS_IMETHODIMP
-nsNavHistory::GetIsFrecencyDecaying(bool* _out) {
- NS_ENSURE_ARG_POINTER(_out);
- *_out = nsNavHistory::sIsFrecencyDecaying;
- return NS_OK;
-}
-
-NS_IMETHODIMP
-nsNavHistory::SetIsFrecencyDecaying(bool aVal) {
- nsNavHistory::sIsFrecencyDecaying = aVal;
- return NS_OK;
-}
-
-NS_IMETHODIMP
nsNavHistory::GetIsAlternativeFrecencyEnabled(bool* _out) {
*_out =
StaticPrefs::places_frecency_pages_alternative_featureGate_AtStartup();
diff --git a/toolkit/components/places/nsNavHistory.h b/toolkit/components/places/nsNavHistory.h
@@ -339,10 +339,6 @@ class nsNavHistory final : public nsSupportsWeakReference,
static mozilla::Atomic<int64_t> sLastInsertedVisitId;
/**
- * Tracks whether frecency is currently being decayed.
- */
- static mozilla::Atomic<bool> sIsFrecencyDecaying;
- /**
* Tracks whether there's frecency to be recalculated.
*/
static mozilla::Atomic<bool> sShouldStartFrecencyRecalculation;
diff --git a/toolkit/components/places/nsPlacesTriggers.h b/toolkit/components/places/nsPlacesTriggers.h
@@ -130,17 +130,10 @@
"END")
// This trigger runs on updates to moz_places.frecency.
-//
-// However, we skip this when frecency changes are due to frecency decay
-// since (1) decay updates all frecencies at once, so this trigger would
-// run for each moz_place, which would be expensive; and (2) decay does
-// not change the ordering of frecencies since all frecencies decay by
-// the same percentage.
# define CREATE_PLACES_AFTERUPDATE_FRECENCY_TRIGGER \
nsLiteralCString( \
"CREATE TEMP TRIGGER moz_places_afterupdate_frecency_trigger " \
"AFTER UPDATE OF frecency ON moz_places FOR EACH ROW " \
- "WHEN NOT is_frecency_decaying() " \
"BEGIN " \
"UPDATE moz_places SET recalc_frecency = 0 WHERE id = NEW.id; " \
"UPDATE moz_origins SET recalc_frecency = 1, recalc_alt_frecency = 1 " \
diff --git a/toolkit/components/places/tests/unit/test_frecency_decay.js b/toolkit/components/places/tests/unit/test_frecency_decay.js
@@ -1,82 +0,0 @@
-/* Any copyright is dedicated to the Public Domain.
- * https://creativecommons.org/publicdomain/zero/1.0/ */
-
-const PREF_FREC_DECAY_RATE_DEF = 0.975;
-
-/**
- * Promises that the pages-rank-changed event has been seen.
- *
- * @returns {Promise} A promise which is resolved when the notification is seen.
- */
-function promiseRankingChanged() {
- return PlacesTestUtils.waitForNotification("pages-rank-changed");
-}
-
-add_task(async function setup() {
- Services.prefs.setCharPref(
- "places.frecency.decayRate",
- PREF_FREC_DECAY_RATE_DEF
- );
-});
-
-add_task(async function test_isFrecencyDecaying() {
- let db = await PlacesUtils.promiseDBConnection();
- async function queryFrecencyDecaying() {
- return (
- await db.executeCached(`SELECT is_frecency_decaying()`)
- )[0].getResultByIndex(0);
- }
- PlacesUtils.history.isFrecencyDecaying = true;
- Assert.equal(PlacesUtils.history.isFrecencyDecaying, true);
- Assert.equal(await queryFrecencyDecaying(), true);
- PlacesUtils.history.isFrecencyDecaying = false;
- Assert.equal(PlacesUtils.history.isFrecencyDecaying, false);
- Assert.equal(await queryFrecencyDecaying(), false);
-});
-
-add_task(async function test_frecency_decay() {
- let unvisitedBookmarkFrecency = Services.prefs.getIntPref(
- "places.frecency.unvisitedBookmarkBonus"
- );
-
- // Add a bookmark and check its frecency.
- let url = "http://example.com/b";
- let promiseOne = promiseRankingChanged();
- await PlacesUtils.bookmarks.insert({
- url,
- parentGuid: PlacesUtils.bookmarks.unfiledGuid,
- });
- await PlacesFrecencyRecalculator.recalculateAnyOutdatedFrecencies();
- await promiseOne;
-
- let histogram = TelemetryTestUtils.getAndClearHistogram(
- "PLACES_IDLE_FRECENCY_DECAY_TIME_MS"
- );
- info("Trigger frecency decay.");
- Assert.equal(PlacesUtils.history.isFrecencyDecaying, false);
- let promiseRanking = promiseRankingChanged();
-
- PlacesFrecencyRecalculator.observe(null, "idle-daily", "");
- Assert.equal(PlacesUtils.history.isFrecencyDecaying, true);
- info("Wait for completion.");
- await PlacesFrecencyRecalculator.pendingFrecencyDecayPromise;
-
- await promiseRanking;
- Assert.equal(PlacesUtils.history.isFrecencyDecaying, false);
-
- // Now check the new frecency is correct.
- let newFrecency = await PlacesTestUtils.getDatabaseValue(
- "moz_places",
- "frecency",
- { url }
- );
-
- Assert.equal(
- newFrecency,
- Math.round(unvisitedBookmarkFrecency * PREF_FREC_DECAY_RATE_DEF),
- "Frecencies should match"
- );
-
- let snapshot = histogram.snapshot();
- Assert.greater(Object.values(snapshot.values).length, 0);
-});
diff --git a/toolkit/components/places/tests/unit/test_frecency_observers.js b/toolkit/components/places/tests/unit/test_frecency_observers.js
@@ -72,16 +72,6 @@ add_task(async function test_clear() {
Assert.deepEqual(received, ["history-cleared"]);
});
-add_task(async function test_nsNavHistory_idleDaily() {
- await PlacesUtils.bookmarks.insert({
- parentGuid: PlacesUtils.bookmarks.unfiledGuid,
- url: "https://test-site1.org",
- title: "test",
- });
- PlacesFrecencyRecalculator.observe(null, "idle-daily", "");
- await Promise.all([onRankingChanged()]);
-});
-
add_task(async function test_nsNavHistory_recalculate() {
await PlacesUtils.bookmarks.insert({
parentGuid: PlacesUtils.bookmarks.unfiledGuid,
diff --git a/toolkit/components/places/tests/unit/xpcshell.toml b/toolkit/components/places/tests/unit/xpcshell.toml
@@ -115,8 +115,6 @@ run-if = ["buildapp == 'browser'"]
["test_childlessTags.js"]
-["test_frecency_decay.js"]
-
["test_frecency_interactions.js"]
prefs = ["places.frecency.pages.alternative.featureGate=true"]
