From f43e9df2c97c0c91b5fb50d7dc60727b8c8f8139 Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Mon, 17 Oct 2022 01:06:41 -0400 Subject: [PATCH 1/2] field_value_android.h: change std::vector and MapFieldValue constructors to take const references, instead of values --- firestore/src/android/field_value_android.cc | 4 ++-- firestore/src/android/field_value_android.h | 10 ++++++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/firestore/src/android/field_value_android.cc b/firestore/src/android/field_value_android.cc index 1096bc475f..071188a726 100644 --- a/firestore/src/android/field_value_android.cc +++ b/firestore/src/android/field_value_android.cc @@ -156,7 +156,7 @@ FieldValueInternal::FieldValueInternal(GeoPoint value) object_ = GeoPointInternal::Create(env, value); } -FieldValueInternal::FieldValueInternal(std::vector value) +FieldValueInternal::FieldValueInternal(const std::vector& value) : cached_type_(Type::kArray) { Env env = GetEnv(); Local list = ArrayList::Create(env, value.size()); @@ -167,7 +167,7 @@ FieldValueInternal::FieldValueInternal(std::vector value) object_ = list; } -FieldValueInternal::FieldValueInternal(MapFieldValue value) +FieldValueInternal::FieldValueInternal(const MapFieldValue& value) : cached_type_(Type::kMap) { Env env = GetEnv(); Local map = HashMap::Create(env); diff --git a/firestore/src/android/field_value_android.h b/firestore/src/android/field_value_android.h index 6ab290d3d5..ceb49d23da 100644 --- a/firestore/src/android/field_value_android.h +++ b/firestore/src/android/field_value_android.h @@ -64,8 +64,14 @@ class FieldValueInternal { FieldValueInternal(const uint8_t* value, size_t size); explicit FieldValueInternal(DocumentReference value); explicit FieldValueInternal(GeoPoint value); - explicit FieldValueInternal(std::vector value); - explicit FieldValueInternal(MapFieldValue value); + // Deviate from the iOS signatures of the following two constructors. The iOS + // versions take values into which the caller moves, to elide a copy. In + // Android, this actually *costs* an extra copy when calling from + // `DocumentReferenceInternal::Set()`, doubling the number of global + // references needed. Using const references in Android avoids this extra, + // costly copy (https://github.com/firebase/quickstart-unity/issues/1303). + explicit FieldValueInternal(const std::vector& value); + explicit FieldValueInternal(const MapFieldValue& value); Type type() const; From 8918ee511b74ed6965b182c890a99ee35a0e0e65 Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Mon, 17 Oct 2022 01:18:22 -0400 Subject: [PATCH 2/2] release_build_files/readme.md: add release notes entry --- release_build_files/readme.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/release_build_files/readme.md b/release_build_files/readme.md index e693ae45da..58e89be7ec 100644 --- a/release_build_files/readme.md +++ b/release_build_files/readme.md @@ -634,6 +634,12 @@ workflow use only during the development of your app, not for publicly shipping code. ## Release Notes +### Upcoming +- Changes + - Firestore (Android): Reduce the number of JNI global references consumed + when creating or updating documents + ([#1111](https://github.com/firebase/firebase-cpp-sdk/pull/1111)). + ### 10.0.0 - Changes - General (Android): Update to Firebase Android BoM version 31.0.0.