From df5a4d63e80ed612c443c95ffad07391a4c9f110 Mon Sep 17 00:00:00 2001 From: Hong Shin Date: Thu, 26 Sep 2024 15:02:48 -0700 Subject: [PATCH] Move MessageLock to message_lock.cc Move hpb::internal::{Serialize, HasExtensionOrUnknown, GetOrPromoteExtension, DeepCopy, DeepClone} to message_lock.h as they all use the message lock absl status helpers are now housed in status.h PiperOrigin-RevId: 679307322 --- hpb/hpb.cc | 78 +--------------------------------- hpb/hpb.h | 42 +----------------- hpb/internal/message_lock.cc | 82 ++++++++++++++++++++++++++++++++++++ hpb/internal/message_lock.h | 20 +++++++++ hpb/status.h | 39 +++++++++++++++++ 5 files changed, 144 insertions(+), 117 deletions(-) create mode 100644 hpb/status.h diff --git a/hpb/hpb.cc b/hpb/hpb.cc index 21eada8acb..d1f55d1622 100644 --- a/hpb/hpb.cc +++ b/hpb/hpb.cc @@ -7,28 +7,19 @@ #include "google/protobuf/hpb/hpb.h" -#include -#include - #include "absl/status/status.h" -#include "absl/status/statusor.h" #include "absl/strings/str_format.h" #include "absl/strings/string_view.h" #include "google/protobuf/hpb/internal/message_lock.h" +#include "google/protobuf/hpb/status.h" #include "upb/mem/arena.h" #include "upb/message/accessors.h" -#include "upb/message/copy.h" #include "upb/message/message.h" -#include "upb/message/promote.h" -#include "upb/message/value.h" #include "upb/mini_table/extension.h" -#include "upb/mini_table/extension_registry.h" -#include "upb/mini_table/message.h" #include "upb/wire/decode.h" #include "upb/wire/encode.h" namespace hpb { - absl::Status MessageAllocationError(SourceLocation loc) { return absl::Status(absl::StatusCode::kUnknown, "Upb message allocation error"); @@ -51,73 +42,6 @@ absl::Status MessageDecodeError(upb_DecodeStatus status, SourceLocation loc namespace internal { -/** - * MessageLock(msg) acquires lock on msg when constructed and releases it when - * destroyed. - */ -class MessageLock { - public: - explicit MessageLock(const upb_Message* msg) : msg_(msg) { - UpbExtensionLocker locker = - upb_extension_locker_global.load(std::memory_order_acquire); - unlocker_ = (locker != nullptr) ? locker(msg) : nullptr; - } - MessageLock(const MessageLock&) = delete; - void operator=(const MessageLock&) = delete; - ~MessageLock() { - if (unlocker_ != nullptr) { - unlocker_(msg_); - } - } - - private: - const upb_Message* msg_; - UpbExtensionUnlocker unlocker_; -}; - -bool HasExtensionOrUnknown(const upb_Message* msg, - const upb_MiniTableExtension* eid) { - MessageLock msg_lock(msg); - if (upb_Message_HasExtension(msg, eid)) return true; - - const int number = upb_MiniTableExtension_Number(eid); - return upb_Message_FindUnknown(msg, number, 0).status == kUpb_FindUnknown_Ok; -} - -bool GetOrPromoteExtension(upb_Message* msg, const upb_MiniTableExtension* eid, - upb_Arena* arena, upb_MessageValue* value) { - MessageLock msg_lock(msg); - upb_GetExtension_Status ext_status = upb_Message_GetOrPromoteExtension( - (upb_Message*)msg, eid, 0, arena, value); - return ext_status == kUpb_GetExtension_Ok; -} - -absl::StatusOr Serialize(const upb_Message* message, - const upb_MiniTable* mini_table, - upb_Arena* arena, int options) { - MessageLock msg_lock(message); - size_t len; - char* ptr; - upb_EncodeStatus status = - upb_Encode(message, mini_table, options, arena, &ptr, &len); - if (status == kUpb_EncodeStatus_Ok) { - return absl::string_view(ptr, len); - } - return MessageEncodeError(status); -} - -void DeepCopy(upb_Message* target, const upb_Message* source, - const upb_MiniTable* mini_table, upb_Arena* arena) { - MessageLock msg_lock(source); - upb_Message_DeepCopy(target, source, mini_table, arena); -} - -upb_Message* DeepClone(const upb_Message* source, - const upb_MiniTable* mini_table, upb_Arena* arena) { - MessageLock msg_lock(source); - return upb_Message_DeepClone(source, mini_table, arena); -} - absl::Status MoveExtension(upb_Message* message, upb_Arena* message_arena, const upb_MiniTableExtension* ext, upb_Message* extension, upb_Arena* extension_arena) { diff --git a/hpb/hpb.h b/hpb/hpb.h index b84b0534ab..f847480537 100644 --- a/hpb/hpb.h +++ b/hpb/hpb.h @@ -10,7 +10,6 @@ #include #include -#include #include "absl/status/status.h" #include "absl/status/statusor.h" @@ -19,14 +18,13 @@ #include "google/protobuf/hpb/backend/upb/interop.h" #include "google/protobuf/hpb/extension.h" #include "google/protobuf/hpb/internal/internal.h" +#include "google/protobuf/hpb/internal/message_lock.h" #include "google/protobuf/hpb/internal/template_help.h" #include "google/protobuf/hpb/ptr.h" -#include "upb/base/status.hpp" +#include "google/protobuf/hpb/status.h" #include "upb/mem/arena.hpp" -#include "upb/message/copy.h" #include "upb/mini_table/extension.h" #include "upb/wire/decode.h" -#include "upb/wire/encode.h" #ifdef HPB_BACKEND_UPB #include "google/protobuf/hpb/backend/upb/upb.h" @@ -37,44 +35,8 @@ namespace hpb { class ExtensionRegistry; -// This type exists to work around an absl type that has not yet been -// released. -struct SourceLocation { - static SourceLocation current() { return {}; } - absl::string_view file_name() { return ""; } - int line() { return 0; } -}; - -absl::Status MessageAllocationError( - SourceLocation loc = SourceLocation::current()); - -absl::Status ExtensionNotFoundError( - int extension_number, SourceLocation loc = SourceLocation::current()); - -absl::Status MessageDecodeError(upb_DecodeStatus status, - SourceLocation loc = SourceLocation::current()); - -absl::Status MessageEncodeError(upb_EncodeStatus status, - SourceLocation loc = SourceLocation::current()); - namespace internal { -absl::StatusOr Serialize(const upb_Message* message, - const upb_MiniTable* mini_table, - upb_Arena* arena, int options); - -bool HasExtensionOrUnknown(const upb_Message* msg, - const upb_MiniTableExtension* eid); - -bool GetOrPromoteExtension(upb_Message* msg, const upb_MiniTableExtension* eid, - upb_Arena* arena, upb_MessageValue* value); - -void DeepCopy(upb_Message* target, const upb_Message* source, - const upb_MiniTable* mini_table, upb_Arena* arena); - -upb_Message* DeepClone(const upb_Message* source, - const upb_MiniTable* mini_table, upb_Arena* arena); - absl::Status MoveExtension(upb_Message* message, upb_Arena* message_arena, const upb_MiniTableExtension* ext, upb_Message* extension, upb_Arena* extension_arena); diff --git a/hpb/internal/message_lock.cc b/hpb/internal/message_lock.cc index 62f69ce157..bc6aa10480 100644 --- a/hpb/internal/message_lock.cc +++ b/hpb/internal/message_lock.cc @@ -8,9 +8,91 @@ #include "google/protobuf/hpb/internal/message_lock.h" #include +#include +#include + +#include "absl/status/statusor.h" +#include "absl/strings/string_view.h" +#include "google/protobuf/hpb/status.h" +#include "upb/mem/arena.h" +#include "upb/message/accessors.h" +#include "upb/message/array.h" +#include "upb/message/copy.h" +#include "upb/message/message.h" +#include "upb/message/promote.h" +#include "upb/mini_table/extension.h" +#include "upb/mini_table/message.h" +#include "upb/wire/encode.h" namespace hpb::internal { std::atomic upb_extension_locker_global; +/** + * MessageLock(msg) acquires lock on msg when constructed and releases it when + * destroyed. + */ +class MessageLock { + public: + explicit MessageLock(const upb_Message* msg) : msg_(msg) { + UpbExtensionLocker locker = + upb_extension_locker_global.load(std::memory_order_acquire); + unlocker_ = (locker != nullptr) ? locker(msg) : nullptr; + } + MessageLock(const MessageLock&) = delete; + void operator=(const MessageLock&) = delete; + ~MessageLock() { + if (unlocker_ != nullptr) { + unlocker_(msg_); + } + } + + private: + const upb_Message* msg_; + UpbExtensionUnlocker unlocker_; +}; + +bool HasExtensionOrUnknown(const upb_Message* msg, + const upb_MiniTableExtension* eid) { + MessageLock msg_lock(msg); + if (upb_Message_HasExtension(msg, eid)) return true; + + const uint32_t number = upb_MiniTableExtension_Number(eid); + return upb_Message_FindUnknown(msg, number, 0).status == kUpb_FindUnknown_Ok; +} + +bool GetOrPromoteExtension(upb_Message* msg, const upb_MiniTableExtension* eid, + upb_Arena* arena, upb_MessageValue* value) { + MessageLock msg_lock(msg); + upb_GetExtension_Status ext_status = upb_Message_GetOrPromoteExtension( + (upb_Message*)msg, eid, 0, arena, value); + return ext_status == kUpb_GetExtension_Ok; +} + +absl::StatusOr Serialize(const upb_Message* message, + const upb_MiniTable* mini_table, + upb_Arena* arena, int options) { + MessageLock msg_lock(message); + size_t len; + char* ptr; + upb_EncodeStatus status = + upb_Encode(message, mini_table, options, arena, &ptr, &len); + if (status == kUpb_EncodeStatus_Ok) { + return absl::string_view(ptr, len); + } + return MessageEncodeError(status); +} + +void DeepCopy(upb_Message* target, const upb_Message* source, + const upb_MiniTable* mini_table, upb_Arena* arena) { + MessageLock msg_lock(source); + upb_Message_DeepCopy(target, source, mini_table, arena); +} + +upb_Message* DeepClone(const upb_Message* source, + const upb_MiniTable* mini_table, upb_Arena* arena) { + MessageLock msg_lock(source); + return upb_Message_DeepClone(source, mini_table, arena); +} + } // namespace hpb::internal diff --git a/hpb/internal/message_lock.h b/hpb/internal/message_lock.h index 430d24bb79..5ec04bd04c 100644 --- a/hpb/internal/message_lock.h +++ b/hpb/internal/message_lock.h @@ -10,6 +10,10 @@ #include +#include "absl/status/statusor.h" +#include "absl/strings/string_view.h" +#include "upb/message/message.h" + namespace hpb::internal { // TODO: Temporary locking api for cross-language @@ -26,6 +30,22 @@ using UpbExtensionLocker = UpbExtensionUnlocker (*)(const void*); // TODO: Expose as function instead of global. extern std::atomic upb_extension_locker_global; +absl::StatusOr Serialize(const upb_Message* message, + const upb_MiniTable* mini_table, + upb_Arena* arena, int options); + +bool HasExtensionOrUnknown(const upb_Message* msg, + const upb_MiniTableExtension* eid); + +bool GetOrPromoteExtension(upb_Message* msg, const upb_MiniTableExtension* eid, + upb_Arena* arena, upb_MessageValue* value); + +void DeepCopy(upb_Message* target, const upb_Message* source, + const upb_MiniTable* mini_table, upb_Arena* arena); + +upb_Message* DeepClone(const upb_Message* source, + const upb_MiniTable* mini_table, upb_Arena* arena); + } // namespace hpb::internal #endif // PROTOBUF_HPB_EXTENSION_LOCK_H_ diff --git a/hpb/status.h b/hpb/status.h new file mode 100644 index 0000000000..5c373098fe --- /dev/null +++ b/hpb/status.h @@ -0,0 +1,39 @@ +// Protocol Buffers - Google's data interchange format +// Copyright 2024 Google LLC. All rights reserved. +// +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file or at +// https://developers.google.com/open-source/licenses/bsd + +#ifndef GOOGLE_PROTOBUF_HPB_STATUS_H__ +#define GOOGLE_PROTOBUF_HPB_STATUS_H__ + +#include "absl/status/status.h" +#include "absl/types/source_location.h" +#include "upb/wire/decode.h" +#include "upb/wire/encode.h" + +namespace hpb { + +// This type exists to work around an absl type that has not yet been +// released. +struct SourceLocation { + static SourceLocation current() { return {}; } + absl::string_view file_name() { return ""; } + int line() { return 0; } +}; + +absl::Status MessageEncodeError(upb_EncodeStatus status, + SourceLocation loc = SourceLocation::current()); + +absl::Status MessageAllocationError( + SourceLocation loc = SourceLocation::current()); + +absl::Status ExtensionNotFoundError( + int extension_number, SourceLocation loc = SourceLocation::current()); + +absl::Status MessageDecodeError(upb_DecodeStatus status, + SourceLocation loc = SourceLocation::current()); +} // namespace hpb + +#endif // GOOGLE_PROTOBUF_HPB_STATUS_H__