From ee50464ccd27efbd57eac97c9a27443ad5040fb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kasper=20Overg=C3=A5rd=20Nielsen?= Date: Mon, 15 Apr 2024 10:55:48 +0200 Subject: [PATCH 1/4] Compare 8 bytes at a time for Uint8Lists --- .../realm_common/lib/src/realm_types.dart | 42 ++++++++++++++++--- 1 file changed, 36 insertions(+), 6 deletions(-) diff --git a/packages/realm_common/lib/src/realm_types.dart b/packages/realm_common/lib/src/realm_types.dart index dcd9233e1..3db629a52 100644 --- a/packages/realm_common/lib/src/realm_types.dart +++ b/packages/realm_common/lib/src/realm_types.dart @@ -187,7 +187,7 @@ enum RealmValueType { bool get isCollection => this == RealmValueType.list || this == RealmValueType.map; } -/// A type that can represent any valid realm data type, except collections and embedded objects. +/// A type that can represent any valid realm data type, except embedded objects. /// /// You can use [RealmValue] to declare fields on realm models, in which case it must be non-nullable, /// but it can wrap a null-value. List of [RealmValue] (`List`) are also legal. @@ -278,21 +278,29 @@ class RealmValue { } @override - operator ==(Object? other) { + operator ==(Object other) { + // TODO(kn): + // Should we not start by testing for identical? Will break current + // test, but I think the tests are wrong + //if (identical(this, other)) return true; + // We always return false when comparing two RealmValue collections. if (type.isCollection) { return false; } + final v = value; + if (other is RealmValue) { - if (value is Uint8List && other.value is Uint8List) { - return ListEquality().equals(value as Uint8List, other.value as Uint8List); + final ov = other.value; + if (v is Uint8List && ov is Uint8List) { + return memEquals(v, ov); } - return type == other.type && value == other.value; + return type == other.type && v == ov; } - return value == other; + return v == other; } @override @@ -302,6 +310,28 @@ class RealmValue { String toString() => 'RealmValue($value)'; } +/// Compares two [Uint8List]s by comparing 8 bytes at a time. +bool memEquals(Uint8List x, Uint8List y) { + if (identical(x, y)) return true; + if (x.lengthInBytes != y.lengthInBytes) return false; + + var words = x.lengthInBytes ~/ 8; // number of full words + var xW = x.buffer.asUint64List(0, words); + var yW = y.buffer.asUint64List(0, words); + + // compare words + for (var i = 0; i < xW.length; i += 1) { + if (xW[i] != yW[i]) return false; + } + + // compare remaining bytes + for (var i = words * 8; i < x.lengthInBytes; i += 1) { + if (x[i] != y[i]) return false; + } + + return true; // no diff, they are equal +} + /// A base type for the supported geospatial shapes. sealed class GeoShape {} From ba4e6fc7a37d1cf5cf6a627d235a56bf70fac083 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kasper=20Overg=C3=A5rd=20Nielsen?= Date: Tue, 16 Apr 2024 15:31:11 +0200 Subject: [PATCH 2/4] Update CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a133af700..7dcbde59a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ * Added support for specifying schema version on `Configuration.flexibleSync`. This allows you to take advantage of an upcoming server-side feature that will allow schema migrations for synchronized Realms. (Issue [#1599](https://github.com/realm/realm-dart/issues/1599)) * The default base url in `AppConfiguration` has been updated to point to `services.cloud.mongodb.com`. See https://www.mongodb.com/docs/atlas/app-services/domain-migration/ for more information. (Issue [#1549](https://github.com/realm/realm-dart/issues/1549)) * Don't ignore private fields on realm models. (Issue [#1367](https://github.com/realm/realm-dart/issues/1367)) +* Improve performance of `RealmValue.operator==` when containing binary data. (PR [#1628](https://github.com/realm/realm-dart/pull/1628)) ### Fixed * Using valid const, but non-literal expressions, such as negation of numbers, as an initializer would fail. (Issue [#1606](https://github.com/realm/realm-dart/issues/1606)) From 9967fb9134aacc9994981ab6749b123fef854928 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kasper=20Overg=C3=A5rd=20Nielsen?= Date: Tue, 16 Apr 2024 17:45:36 +0200 Subject: [PATCH 3/4] Add test to evaluate correctnes and performance of memEquals --- .../realm_common/lib/src/realm_types.dart | 1 - .../realm_dart/test/realm_value_test.dart | 52 +++++++++++++++++-- 2 files changed, 49 insertions(+), 4 deletions(-) diff --git a/packages/realm_common/lib/src/realm_types.dart b/packages/realm_common/lib/src/realm_types.dart index 3db629a52..296c3f647 100644 --- a/packages/realm_common/lib/src/realm_types.dart +++ b/packages/realm_common/lib/src/realm_types.dart @@ -6,7 +6,6 @@ import 'dart:math'; import 'dart:typed_data'; import 'package:objectid/objectid.dart'; import 'package:sane_uuid/uuid.dart'; -import 'package:collection/collection.dart'; Type _typeOf() => T; diff --git a/packages/realm_dart/test/realm_value_test.dart b/packages/realm_dart/test/realm_value_test.dart index 50579ea9c..5cd921ad8 100644 --- a/packages/realm_dart/test/realm_value_test.dart +++ b/packages/realm_dart/test/realm_value_test.dart @@ -1,13 +1,16 @@ // Copyright 2022 MongoDB, Inc. // SPDX-License-Identifier: Apache-2.0 +import 'dart:math'; import 'dart:typed_data'; -import 'package:test/test.dart' hide test, throws; +import 'package:collection/collection.dart'; +import 'package:realm_common/realm_common.dart' show memEquals; import 'package:realm_dart/realm.dart'; +import 'package:test/test.dart' hide test, throws; -import 'test.dart'; import 'session_test.dart' show validateSessionStates; +import 'test.dart'; part 'realm_value_test.realm.dart'; @@ -60,7 +63,7 @@ void main() { ObjectId.fromHexString('64c13ab08edf48a008793cac'), Uuid.fromString('7a459a5e-5eb6-45f6-9b72-8f794e324105'), Decimal128.fromDouble(128.128), - Uint8List.fromList([1, 2, 0]) + Uint8List.fromList(List.generate(21, (index) => index)), // 21 % 8 != 0 so not a multiple of 8 bytes ]; for (final x in primitiveValues) { @@ -2418,4 +2421,47 @@ void main() { expect(result.value, isNull); }); }); + + test('memEquals', () { + const samples = 100; + const max = 100000; + const half = max ~/ 2; + final listEquals = ListEquality().equals; + + final memEqualsClock = Stopwatch(); + final listEqualsClock = Stopwatch(); + for (int i = 0; i < samples; i++) { + final rnd = Random(i); + final length = rnd.nextInt(half) + half; // 50-100 KB. + final list = List.generate(length, (index) => index)..shuffle(rnd); + final bin1 = Uint8List.fromList(list); + final bin2 = Uint8List.fromList(list); + + // Check correctness. + expect(identical(bin1, bin2), isFalse); // Different instances. + expect(listEquals(bin1, bin2), isTrue); // Sanity check. + expect(memEquals(bin1, bin2), isTrue); // Check correctness. + expect(RealmValue.from(bin1), RealmValue.from(bin2)); + + // Measure performance. + listEqualsClock.start(); + final resultListEquals = listEquals(bin1, bin2); + listEqualsClock.stop(); + + memEqualsClock.start(); + final resultMemEquals = memEquals(bin1, bin2); + memEqualsClock.stop(); + + expect(resultListEquals, resultMemEquals); // Sanity check. + + bin2[rnd.nextInt(bin2.length)]--; // Change one byte. + + expect(listEquals(bin1, bin2), isFalse); // Sanity check. + expect(memEquals(bin1, bin2), isFalse); // Check correctness. + expect(RealmValue.from(bin1), isNot(RealmValue.from(bin2))); + } + + // Not quite 8 times speedup, but close enough. + expect(listEqualsClock.elapsedTicks ~/ 7, greaterThan(memEqualsClock.elapsedTicks)); + }); } From 00858f50540fefebb420d112bd95b485efeea744 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kasper=20Overg=C3=A5rd=20Nielsen?= Date: Tue, 16 Apr 2024 18:03:23 +0200 Subject: [PATCH 4/4] A bit more humility --- packages/realm_dart/test/realm_value_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/realm_dart/test/realm_value_test.dart b/packages/realm_dart/test/realm_value_test.dart index 5cd921ad8..c1d3e5d29 100644 --- a/packages/realm_dart/test/realm_value_test.dart +++ b/packages/realm_dart/test/realm_value_test.dart @@ -2462,6 +2462,6 @@ void main() { } // Not quite 8 times speedup, but close enough. - expect(listEqualsClock.elapsedTicks ~/ 7, greaterThan(memEqualsClock.elapsedTicks)); + expect(listEqualsClock.elapsedTicks ~/ 5, greaterThan(memEqualsClock.elapsedTicks)); }); }