Rust protobuf: remove the need for a generated `placement_new` thunk

We have been relying on a per-message generated `placement_new` function for
implementing map insertion, but this CL simplifies things by removing that.
Instead, we do a reflective swap if possible, or else fall back on a copy.

This will probably make insertions a bit slower, but I think it may be worth it
because it should make it much simpler to have a blanket implementation for
ProxedInMapValue that works for all map types.

It looks like it should be possible to make this faster in the future by
implementing a bitwise move that will work for any message.

PiperOrigin-RevId: 676495920
pull/18314/head
Adam Cozzette 2 months ago committed by Copybara-Service
parent 8681742a14
commit 5c3d1e8c30
  1. 1
      rust/cpp.rs
  2. 25
      rust/cpp_kernel/map.cc
  3. 9
      src/google/protobuf/compiler/rust/message.cc
  4. 5
      src/google/protobuf/map.h

@ -812,7 +812,6 @@ macro_rules! impl_map_primitives {
size_info: MapNodeSizeInfo, size_info: MapNodeSizeInfo,
key: $cpp_type, key: $cpp_type,
value: RawMessage, value: RawMessage,
placement_new: unsafe extern "C" fn(*mut c_void, m: RawMessage),
) -> bool; ) -> bool;
pub fn $get_thunk( pub fn $get_thunk(
m: RawMap, m: RawMap,

@ -7,6 +7,7 @@
#include <utility> #include <utility>
#include "google/protobuf/map.h" #include "google/protobuf/map.h"
#include "google/protobuf/message.h"
#include "google/protobuf/message_lite.h" #include "google/protobuf/message_lite.h"
#include "rust/cpp_kernel/strings.h" #include "rust/cpp_kernel/strings.h"
@ -42,16 +43,27 @@ void DestroyMapNode(internal::UntypedMapBase* m, internal::NodeBase* node,
template <typename Key> template <typename Key>
bool Insert(internal::UntypedMapBase* m, internal::MapNodeSizeInfoT size_info, bool Insert(internal::UntypedMapBase* m, internal::MapNodeSizeInfoT size_info,
Key key, MessageLite* value, Key key, MessageLite* value) {
void (*placement_new)(void*, MessageLite*)) {
internal::NodeBase* node = internal::RustMapHelper::AllocNode(m, size_info); internal::NodeBase* node = internal::RustMapHelper::AllocNode(m, size_info);
if constexpr (std::is_same<Key, PtrAndLen>::value) { if constexpr (std::is_same<Key, PtrAndLen>::value) {
new (node->GetVoidKey()) std::string(key.ptr, key.len); new (node->GetVoidKey()) std::string(key.ptr, key.len);
} else { } else {
*static_cast<Key*>(node->GetVoidKey()) = key; *static_cast<Key*>(node->GetVoidKey()) = key;
} }
void* value_ptr = node->GetVoidValue(size_info);
placement_new(value_ptr, value); MessageLite* new_msg = internal::RustMapHelper::PlacementNew(
value, node->GetVoidValue(size_info));
auto* full_msg = DynamicCastMessage<Message>(new_msg);
// If we are working with a full (non-lite) proto, we reflectively swap the
// value into place. Otherwise, we have to perform a copy.
if (full_msg != nullptr) {
full_msg->GetReflection()->Swap(full_msg,
DynamicCastMessage<Message>(value));
} else {
new_msg->CheckTypeAndMergeFrom(*value);
}
node = internal::RustMapHelper::InsertOrReplaceNode( node = internal::RustMapHelper::InsertOrReplaceNode(
static_cast<KeyMap<Key>*>(m), node); static_cast<KeyMap<Key>*>(m), node);
if (node == nullptr) { if (node == nullptr) {
@ -170,9 +182,8 @@ google::protobuf::internal::UntypedMapIterator proto2_rust_map_iter(
bool proto2_rust_map_insert_##suffix( \ bool proto2_rust_map_insert_##suffix( \
google::protobuf::internal::UntypedMapBase* m, \ google::protobuf::internal::UntypedMapBase* m, \
google::protobuf::internal::MapNodeSizeInfoT size_info, cpp_type key, \ google::protobuf::internal::MapNodeSizeInfoT size_info, cpp_type key, \
google::protobuf::MessageLite* value, \ google::protobuf::MessageLite* value) { \
void (*placement_new)(void*, google::protobuf::MessageLite*)) { \ return google::protobuf::rust::Insert(m, size_info, key, value); \
return google::protobuf::rust::Insert(m, size_info, key, value, placement_new); \
} \ } \
\ \
bool proto2_rust_map_get_##suffix( \ bool proto2_rust_map_get_##suffix( \

@ -195,7 +195,6 @@ void CppMessageExterns(Context& ctx, const Descriptor& msg) {
ABSL_CHECK(ctx.is_cpp()); ABSL_CHECK(ctx.is_cpp());
ctx.Emit( ctx.Emit(
{{"new_thunk", ThunkName(ctx, msg, "new")}, {{"new_thunk", ThunkName(ctx, msg, "new")},
{"placement_new_thunk", ThunkName(ctx, msg, "placement_new")},
{"repeated_new_thunk", ThunkName(ctx, msg, "repeated_new")}, {"repeated_new_thunk", ThunkName(ctx, msg, "repeated_new")},
{"repeated_free_thunk", ThunkName(ctx, msg, "repeated_free")}, {"repeated_free_thunk", ThunkName(ctx, msg, "repeated_free")},
{"repeated_len_thunk", ThunkName(ctx, msg, "repeated_len")}, {"repeated_len_thunk", ThunkName(ctx, msg, "repeated_len")},
@ -208,7 +207,6 @@ void CppMessageExterns(Context& ctx, const Descriptor& msg) {
{"map_size_info_thunk", ThunkName(ctx, msg, "size_info")}}, {"map_size_info_thunk", ThunkName(ctx, msg, "size_info")}},
R"rs( R"rs(
fn $new_thunk$() -> $pbr$::RawMessage; fn $new_thunk$() -> $pbr$::RawMessage;
fn $placement_new_thunk$(ptr: *mut $std$::ffi::c_void, m: $pbr$::RawMessage);
fn $repeated_new_thunk$() -> $pbr$::RawRepeatedField; fn $repeated_new_thunk$() -> $pbr$::RawRepeatedField;
fn $repeated_free_thunk$(raw: $pbr$::RawRepeatedField); fn $repeated_free_thunk$(raw: $pbr$::RawRepeatedField);
fn $repeated_len_thunk$(raw: $pbr$::RawRepeatedField) -> usize; fn $repeated_len_thunk$(raw: $pbr$::RawRepeatedField) -> usize;
@ -566,7 +564,6 @@ void MessageProxiedInMapValue(Context& ctx, const Descriptor& msg) {
for (const auto& t : kMapKeyTypes) { for (const auto& t : kMapKeyTypes) {
ctx.Emit( ctx.Emit(
{{"map_size_info_thunk", ThunkName(ctx, msg, "size_info")}, {{"map_size_info_thunk", ThunkName(ctx, msg, "size_info")},
{"placement_new_thunk", ThunkName(ctx, msg, "placement_new")},
{"map_insert", {"map_insert",
absl::StrCat("proto2_rust_map_insert_", t.thunk_ident)}, absl::StrCat("proto2_rust_map_insert_", t.thunk_ident)},
{"map_remove", {"map_remove",
@ -616,7 +613,7 @@ void MessageProxiedInMapValue(Context& ctx, const Descriptor& msg) {
map.as_raw($pbi$::Private), map.as_raw($pbi$::Private),
$map_size_info_thunk$($key_t$::SIZE_INFO_INDEX), $map_size_info_thunk$($key_t$::SIZE_INFO_INDEX),
$key_expr$, $key_expr$,
value.into_proxied($pbi$::Private).raw_msg(), $placement_new_thunk$) value.into_proxied($pbi$::Private).raw_msg())
} }
} }
@ -1355,7 +1352,6 @@ void GenerateThunksCc(Context& ctx, const Descriptor& msg) {
{"Msg", RsSafeName(msg.name())}, {"Msg", RsSafeName(msg.name())},
{"QualifiedMsg", cpp::QualifiedClassName(&msg)}, {"QualifiedMsg", cpp::QualifiedClassName(&msg)},
{"new_thunk", ThunkName(ctx, msg, "new")}, {"new_thunk", ThunkName(ctx, msg, "new")},
{"placement_new_thunk", ThunkName(ctx, msg, "placement_new")},
{"repeated_new_thunk", ThunkName(ctx, msg, "repeated_new")}, {"repeated_new_thunk", ThunkName(ctx, msg, "repeated_new")},
{"repeated_free_thunk", ThunkName(ctx, msg, "repeated_free")}, {"repeated_free_thunk", ThunkName(ctx, msg, "repeated_free")},
{"repeated_len_thunk", ThunkName(ctx, msg, "repeated_len")}, {"repeated_len_thunk", ThunkName(ctx, msg, "repeated_len")},
@ -1391,9 +1387,6 @@ void GenerateThunksCc(Context& ctx, const Descriptor& msg) {
// clang-format off // clang-format off
extern $abi$ { extern $abi$ {
void* $new_thunk$() { return new $QualifiedMsg$(); } void* $new_thunk$() { return new $QualifiedMsg$(); }
void $placement_new_thunk$(void* ptr, $QualifiedMsg$& m) {
new (ptr) $QualifiedMsg$(std::move(m));
}
void* $repeated_new_thunk$() { void* $repeated_new_thunk$() {
return new google::protobuf::RepeatedPtrField<$QualifiedMsg$>(); return new google::protobuf::RepeatedPtrField<$QualifiedMsg$>();

@ -1190,6 +1190,11 @@ class RustMapHelper {
m->erase_no_destroy(bucket, static_cast<typename Map::KeyNode*>(node)); m->erase_no_destroy(bucket, static_cast<typename Map::KeyNode*>(node));
} }
static google::protobuf::MessageLite* PlacementNew(const MessageLite* prototype,
void* mem) {
return prototype->GetClassData()->PlacementNew(mem, /* arena = */ nullptr);
}
static void DestroyMessage(MessageLite* m) { m->DestroyInstance(); } static void DestroyMessage(MessageLite* m) { m->DestroyInstance(); }
static void ClearTable(UntypedMapBase* m, ClearInput input) { static void ClearTable(UntypedMapBase* m, ClearInput input) {

Loading…
Cancel
Save