Ignore put queries that don't change anything.

Telegram/SourceFiles/storage/cache/storage_cache_database.cpp

@@ -279,6 +279,7 @@ private:
 		EncryptionKey &key);
 	bool readHeader();
 	bool writeHeader();
+
 	void readBinlog();
 	size_type readBinlogRecords(bytes::const_span data);
 	size_type readBinlogRecordSize(bytes::const_span data) const;
@@ -316,6 +317,8 @@ private:
 
 	void setMapEntry(const Key &key, Entry &&entry);
 	void eraseMapEntry(const Map::const_iterator &i);
+	void recordEntryAccess(const Key &key);
+	QByteArray readValueData(PlaceId place, size_type size) const;
 
 	Version findAvailableVersion() const;
 	QString versionPath() const;
@@ -324,13 +327,17 @@ private:
 
 	QString placePath(PlaceId place) const;
 	bool isFreePlace(PlaceId place) const;
+
 	template <typename StoreRecord>
-	QString writeKeyPlaceGeneric(
+	base::optional<QString> writeKeyPlaceGeneric(
 		StoreRecord &&record,
 		const Key &key,
-		size_type size,
+		const QByteArray &value,
+		uint32 checksum);
+	base::optional<QString> writeKeyPlace(
+		const Key &key,
+		const QByteArray &value,
 		uint32 checksum);
-	QString writeKeyPlace(const Key &key, size_type size, uint32 checksum);
 	void writeMultiRemoveLazy();
 	void writeMultiRemove();
 	void writeMultiAccessLazy();
@@ -953,11 +960,17 @@ void Database::put(
 	_removing.erase(key);
 
 	const auto checksum = CountChecksum(bytes::make_span(value));
-	const auto path = writeKeyPlace(key, value.size(), checksum);
+	const auto maybepath = writeKeyPlace(key, value, checksum);
-	if (path.isEmpty()) {
+	if (!maybepath) {
 		invokeCallback(done, ioError(binlogPath()));
 		return;
+	} else if (maybepath->isEmpty()) {
+		// Nothing changed.
+		invokeCallback(done, Error::NoError());
+		recordEntryAccess(key);
+		return;
 	}
+	const auto path = *maybepath;
 	File data;
 	const auto result = data.open(path, File::Mode::Write, _key);
 	switch (result) {
@@ -987,18 +1000,26 @@ void Database::put(
 }
 
 template <typename StoreRecord>
-QString Database::writeKeyPlaceGeneric(
+base::optional<QString> Database::writeKeyPlaceGeneric(
 		StoreRecord &&record,
 		const Key &key,
-		size_type size,
+		const QByteArray &value,
 		uint32 checksum) {
-	Expects(size <= _settings.maxDataSize);
+	Expects(value.size() <= _settings.maxDataSize);
 
+	const auto size = size_type(value.size());
 	record.key = key;
 	record.size = ReadTo<EntrySize>(size);
 	record.checksum = checksum;
 	if (const auto i = _map.find(key); i != end(_map)) {
-		record.place = i->second.place;
+		const auto &already = i->second;
+		if (already.tag == record.tag
+			&& already.size == size
+			&& already.checksum == checksum
+			&& readValueData(already.place, size) == value) {
+			return QString();
+		}
+		record.place = already.place;
 	} else {
 		do {
 			bytes::set_random(bytes::object_as_span(&record.place));
@@ -1015,12 +1036,12 @@ QString Database::writeKeyPlaceGeneric(
 	return result;
 }
 
-QString Database::writeKeyPlace(
+base::optional<QString> Database::writeKeyPlace(
 		const Key &key,
-		size_type size,
+		const QByteArray &data,
 		uint32 checksum) {
 	if (!_settings.trackEstimatedTime) {
-		return writeKeyPlaceGeneric(Store(), key, size, checksum);
+		return writeKeyPlaceGeneric(Store(), key, data, checksum);
 	}
 	auto record = StoreWithTime();
 	record.time = countTimePoint();
@@ -1031,7 +1052,7 @@ QString Database::writeKeyPlace(
 		record.time.system = _latestSystemTime;
 		record.time.relativeAdvancement = 0;
 	}
-	return writeKeyPlaceGeneric(std::move(record), key, size, checksum);
+	return writeKeyPlaceGeneric(std::move(record), key, data, checksum);
 }
 
 void Database::get(const Key &key, FnMut<void(QByteArray)> done) {
@@ -1046,38 +1067,46 @@ void Database::get(const Key &key, FnMut<void(QByteArray)> done) {
 	}
 	const auto &entry = i->second;
 
-	const auto path = placePath(entry.place);
+	auto result = readValueData(entry.place, entry.size);
+	if (result.isEmpty()) {
+		invokeCallback(done, QByteArray());
+	} else if (CountChecksum(bytes::make_span(result)) != entry.checksum) {
+		invokeCallback(done, QByteArray());
+	} else {
+		invokeCallback(done, std::move(result));
+		recordEntryAccess(key);
+	}
+}
+
+QByteArray Database::readValueData(PlaceId place, size_type size) const {
+	const auto path = placePath(place);
 	File data;
 	const auto result = data.open(path, File::Mode::Read, _key);
 	switch (result) {
 	case File::Result::Failed:
-		invokeCallback(done, QByteArray());
+	case File::Result::WrongKey: return QByteArray();
-		break;
-
-	case File::Result::WrongKey:
-		invokeCallback(done, QByteArray());
-		break;
-
 	case File::Result::Success: {
-		auto result = QByteArray(entry.size, Qt::Uninitialized);
+		auto result = QByteArray(size, Qt::Uninitialized);
 		const auto bytes = bytes::make_span(result);
 		const auto read = data.readWithPadding(bytes);
-		if (read != entry.size || CountChecksum(bytes) != entry.checksum) {
+		if (read != size) {
-			invokeCallback(done, QByteArray());
+			return QByteArray();
-		} else {
-			invokeCallback(done, std::move(result));
-			if (_settings.trackEstimatedTime) {
-				_accessed.emplace(key);
-				writeMultiAccessLazy();
-			}
-			startDelayedPruning();
 		}
+		return result;
 	} break;
-
 	default: Unexpected("Result in Database::get.");
 	}
 }
 
+void Database::recordEntryAccess(const Key &key) {
+	if (!_settings.trackEstimatedTime) {
+		return;
+	}
+	_accessed.emplace(key);
+	writeMultiAccessLazy();
+	startDelayedPruning();
+}
+
 void Database::remove(const Key &key, FnMut<void()> done) {
 	const auto i = _map.find(key);
 	if (i != _map.end()) {

Telegram/SourceFiles/storage/cache/storage_cache_database_tests.cpp

@@ -74,6 +74,7 @@ const auto GetValue = [](QByteArray value) {
 
 const auto Settings = [] {
 	auto result = Database::Settings();
+	result.trackEstimatedTime = false;
 	result.writeBundleDelay = 1 * crl::time_type(1000);
 	result.pruneTimeout = 1 * crl::time_type(1500);
 	result.maxDataSize = 20;
@@ -151,6 +152,42 @@ TEST_CASE("encrypted cache db", "[storage_cache_database]") {
 		Semaphore.acquire();
 		REQUIRE((Value == TestValue2));
 
+		db.close([&] { Semaphore.release(); });
+		Semaphore.acquire();
+	}
+	SECTION("overwriting values") {
+		Database db(name, Settings);
+
+		db.open(key, GetResult);
+		Semaphore.acquire();
+		REQUIRE(Result.type == Error::Type::None);
+
+		const auto path = GetBinlogPath();
+
+		db.get(Key{ 0, 1 }, GetValue);
+		Semaphore.acquire();
+		REQUIRE((Value == TestValue1));
+
+		const auto size = QFile(path).size();
+
+		db.put(Key{ 0, 1 }, TestValue2, GetResult);
+		Semaphore.acquire();
+		REQUIRE(Result.type == Error::Type::None);
+
+		const auto next = QFile(path).size();
+		REQUIRE(next > size);
+
+		db.get(Key{ 0, 1 }, GetValue);
+		Semaphore.acquire();
+		REQUIRE((Value == TestValue2));
+
+		db.put(Key{ 0, 1 }, TestValue2, GetResult);
+		Semaphore.acquire();
+		REQUIRE(Result.type == Error::Type::None);
+
+		const auto same = QFile(path).size();
+		REQUIRE(same == next);
+
 		db.close([&] { Semaphore.release(); });
 		Semaphore.acquire();
 	}
@@ -206,7 +243,9 @@ TEST_CASE("cache db remove", "[storage_cache_database]") {
 
 TEST_CASE("cache db bundled actions", "[storage_cache_database]") {
 	SECTION("db touched written lazily") {
-		Database db(name, Settings);
+		auto settings = Settings;
+		settings.trackEstimatedTime = true;
+		Database db(name, settings);
 
 		db.clear(GetResult);
 		Semaphore.acquire();
@@ -240,7 +279,9 @@ TEST_CASE("cache db bundled actions", "[storage_cache_database]") {
 		Semaphore.acquire();
 	}
 	SECTION("db touched written on close") {
-		Database db(name, Settings);
+		auto settings = Settings;
+		settings.trackEstimatedTime = true;
+		Database db(name, settings);
 
 		db.clear(GetResult);
 		Semaphore.acquire();
@@ -340,6 +381,7 @@ TEST_CASE("cache db bundled actions", "[storage_cache_database]") {
 TEST_CASE("cache db limits", "[storage_cache_database]") {
 	SECTION("db both limit") {
 		auto settings = Settings;
+		settings.trackEstimatedTime = true;
 		settings.totalSizeLimit = 17 * 3 + 1;
 		settings.totalTimeLimit = 4;
 		Database db(name, settings);
@@ -376,6 +418,7 @@ TEST_CASE("cache db limits", "[storage_cache_database]") {
 	}
 	SECTION("db size limit") {
 		auto settings = Settings;
+		settings.trackEstimatedTime = true;
 		settings.totalSizeLimit = 17 * 3 + 1;
 		Database db(name, settings);
 
@@ -442,6 +485,7 @@ TEST_CASE("cache db limits", "[storage_cache_database]") {
 	}
 	SECTION("db time limit") {
 		auto settings = Settings;
+		settings.trackEstimatedTime = true;
 		settings.totalTimeLimit = 3;
 		Database db(name, settings);