From 4dbce4275bd2b797c14414bb76ea577eae11d835 Mon Sep 17 00:00:00 2001 From: Nicholas Van Sickle Date: Thu, 2 Dec 2021 17:46:07 -0800 Subject: [PATCH] Refactor the interface after some chatting with @amazon-employee-dm Signed-off-by: Nicholas Van Sickle --- .../AzCore/DOM/Backends/JSON/JsonBackend.h | 19 +++++----- .../Backends/JSON/JsonSerializationUtils.h | 17 ++++----- .../AzCore/AzCore/DOM/DomBackend.cpp | 36 ++----------------- Code/Framework/AzCore/AzCore/DOM/DomBackend.h | 20 ++++------- .../AzCore/AzCore/azcore_files.cmake | 2 ++ .../AzCore/Tests/DOM/DomJsonBenchmarks.cpp | 5 +-- .../AzCore/Tests/DOM/DomJsonTests.cpp | 11 +++--- 7 files changed, 41 insertions(+), 69 deletions(-) diff --git a/Code/Framework/AzCore/AzCore/DOM/Backends/JSON/JsonBackend.h b/Code/Framework/AzCore/AzCore/DOM/Backends/JSON/JsonBackend.h index ace21d9be8..1c8f4eb607 100644 --- a/Code/Framework/AzCore/AzCore/DOM/Backends/JSON/JsonBackend.h +++ b/Code/Framework/AzCore/AzCore/DOM/Backends/JSON/JsonBackend.h @@ -8,31 +8,34 @@ #pragma once -#include #include +#include namespace AZ::Dom { //! A DOM backend for serializing and deserializing JSON <=> UTF-8 text //! \param ParseFlags Controls how deserialized JSON is parsed. //! \param WriteFormat Controls how serialized JSON is formatted. - template + template< + Json::ParseFlags ParseFlags = Json::ParseFlags::ParseComments, + Json::OutputFormatting WriteFormat = Json::OutputFormatting::PrettyPrintedJson> class JsonBackend final : public Backend { public: - Visitor::Result ReadFromStringInPlace(AZStd::string& buffer, Visitor& visitor) override + Visitor::Result ReadFromBuffer(const char* buffer, size_t size, AZ::Dom::Lifetime lifetime, Visitor& visitor) override { - return Json::VisitSerializedJsonInPlace(buffer, visitor); + return Json::VisitSerializedJson({ buffer, size }, lifetime, visitor); } - Visitor::Result ReadFromString(AZStd::string_view buffer, Lifetime lifetime, Visitor& visitor) override + Visitor::Result ReadFromBufferInPlace(char* buffer, Visitor& visitor) override { - return Json::VisitSerializedJson(buffer, lifetime, visitor); + return Json::VisitSerializedJsonInPlace(buffer, visitor); } - AZStd::unique_ptr CreateStreamWriter(AZ::IO::GenericStream& stream) override + Visitor::Result WriteToStream(AZ::IO::GenericStream& stream, WriteCallback callback) override { - return Json::CreateJsonStreamWriter(stream, WriteFormat); + AZStd::unique_ptr visitor = Json::CreateJsonStreamWriter(stream, WriteFormat); + return callback(*visitor.get()); } }; } // namespace AZ::Dom diff --git a/Code/Framework/AzCore/AzCore/DOM/Backends/JSON/JsonSerializationUtils.h b/Code/Framework/AzCore/AzCore/DOM/Backends/JSON/JsonSerializationUtils.h index 88c6c42978..db43395475 100644 --- a/Code/Framework/AzCore/AzCore/DOM/Backends/JSON/JsonSerializationUtils.h +++ b/Code/Framework/AzCore/AzCore/DOM/Backends/JSON/JsonSerializationUtils.h @@ -115,17 +115,17 @@ namespace AZ::Dom::Json //! rapidjson stream wrapper for AZStd::string suitable for in-situ parsing //! Faster than rapidjson::MemoryStream for reading from AZStd::string / AZStd::string_view (because it requires a null terminator) //! \note This needs to be inlined for performance reasons. - struct AzStringStream + struct NullDelimitedStringStream { using Ch = char; //(buffer.data()); @@ -193,6 +193,7 @@ namespace AZ::Dom::Json //! \return The aggregate result specifying whether the visitor operations were successful. template Visitor::Result VisitSerializedJson(AZStd::string_view buffer, Lifetime lifetime, Visitor& visitor); + //! Reads serialized JSON from a string in-place and applies it to a visitor. //! \param buffer The UTF-8 serialized JSON to read. This buffer will be modified as part of the deserialization process to //! apply null terminators. @@ -201,7 +202,7 @@ namespace AZ::Dom::Json //! \param parseFlags (template) Settings for adjusting parser behavior. //! \return The aggregate result specifying whether the visitor operations were successful. template - Visitor::Result VisitSerializedJsonInPlace(AZStd::string& buffer, Visitor& visitor); + Visitor::Result VisitSerializedJsonInPlace(char* buffer, Visitor& visitor); //! Takes a visitor specified by a callback and produces a rapidjson::Document. //! \param writeCallback A callback specifying a visitor to accept to build the resulting document. @@ -231,7 +232,7 @@ namespace AZ::Dom::Json // If the string is null terminated, we can use the faster AzStringStream path - otherwise we fall back on rapidjson::MemoryStream if (buffer.data()[buffer.size()] == '\0') { - AzStringStream stream(buffer); + NullDelimitedStringStream stream(buffer); reader.Parse(parseFlags)>(stream, handler); } else @@ -243,10 +244,10 @@ namespace AZ::Dom::Json } template - Visitor::Result VisitSerializedJsonInPlace(AZStd::string& buffer, Visitor& visitor) + Visitor::Result VisitSerializedJsonInPlace(char* buffer, Visitor& visitor) { rapidjson::Reader reader; - AzStringStream stream(buffer); + NullDelimitedStringStream stream(buffer); RapidJsonReadHandler handler(&visitor, Lifetime::Persistent); reader.Parse(parseFlags) | rapidjson::kParseInsituFlag>(stream, handler); diff --git a/Code/Framework/AzCore/AzCore/DOM/DomBackend.cpp b/Code/Framework/AzCore/AzCore/DOM/DomBackend.cpp index 258975eff3..8ff0e8a0bb 100644 --- a/Code/Framework/AzCore/AzCore/DOM/DomBackend.cpp +++ b/Code/Framework/AzCore/AzCore/DOM/DomBackend.cpp @@ -8,40 +8,10 @@ #include -#include -#include -#include - namespace AZ::Dom { - Visitor::Result Backend::ReadFromStream(AZ::IO::GenericStream* stream, Visitor& visitor, size_t maxSize) - { - size_t length = stream->GetLength(); - if (length > maxSize) - { - return AZ::Failure(VisitorError(VisitorErrorCode::InternalError, "Stream is too large.")); - } - AZStd::string buffer; - buffer.resize(length); - stream->Read(length, buffer.data()); - return ReadFromString(buffer, Lifetime::Temporary, visitor); - } - - Visitor::Result Backend::ReadFromStringInPlace(AZStd::string& buffer, Visitor& visitor) - { - return ReadFromString(buffer, Lifetime::Persistent, visitor); - } - - Visitor::Result Backend::WriteToStream(AZ::IO::GenericStream& stream, WriteCallback callback) - { - AZStd::unique_ptr writer = CreateStreamWriter(stream); - return callback(*writer.get()); - } - - Visitor::Result Backend::WriteToString(AZStd::string& buffer, WriteCallback callback) + Visitor::Result Backend::ReadFromBufferInPlace(char* buffer, Visitor& visitor) { - AZ::IO::ByteContainerStream stream{&buffer}; - AZStd::unique_ptr writer = CreateStreamWriter(stream); - return callback(*writer.get()); + return ReadFromBuffer(buffer, strlen(buffer), AZ::Dom::Lifetime::Persistent, visitor); } -} +} // namespace AZ::Dom diff --git a/Code/Framework/AzCore/AzCore/DOM/DomBackend.h b/Code/Framework/AzCore/AzCore/DOM/DomBackend.h index d1eee5d676..c70af72652 100644 --- a/Code/Framework/AzCore/AzCore/DOM/DomBackend.h +++ b/Code/Framework/AzCore/AzCore/DOM/DomBackend.h @@ -22,28 +22,22 @@ namespace AZ::Dom public: virtual ~Backend() = default; - //! Attempt to read this format from the given stream into the target Visitor. - //! The base implementation reads the stream into memory and calls ReadFromString. - virtual Visitor::Result ReadFromStream( - AZ::IO::GenericStream* stream, Visitor& visitor, size_t maxSize = AZStd::numeric_limits::max()); + //! Attempt to read this format from the given buffer into the target Visitor. + virtual Visitor::Result ReadFromBuffer( + const char* buffer, size_t size, AZ::Dom::Lifetime lifetime, Visitor& visitor) = 0; //! Attempt to read this format from a mutable string into the target Visitor. This enables some backends to //! parse without making additional string allocations. + //! This string must be null terminated. //! This string may be modified and read in place without being copied, so when calling this please ensure that: //! - The string won't be deallocated until the visitor no longer needs the values and //! - The string is safe to modify in place. - //! The base implementation simply calls ReadFromString. - virtual Visitor::Result ReadFromStringInPlace(AZStd::string& buffer, Visitor& visitor); - //! Attempt to read this format from an immutable buffer in memory into the target Visitor. - virtual Visitor::Result ReadFromString(AZStd::string_view buffer, Lifetime lifetime, Visitor& visitor) = 0; + //! The base implementation simply calls ReadFromBuffer. + virtual Visitor::Result ReadFromBufferInPlace(char* buffer, Visitor& visitor); - //! Acquire a visitor interface for writing to the target output file. - virtual AZStd::unique_ptr CreateStreamWriter(AZ::IO::GenericStream& stream) = 0; //! A callback that accepts a Visitor, making DOM calls to inform the serializer, and returns an //! aggregate error code to indicate whether or not the operation succeeded. using WriteCallback = AZStd::function; //! Attempt to write a value to a stream using a write callback. - Visitor::Result WriteToStream(AZ::IO::GenericStream& stream, WriteCallback callback); - //! Attempt to write a value to a string using a write callback. - Visitor::Result WriteToString(AZStd::string& buffer, WriteCallback callback); + virtual Visitor::Result WriteToStream(AZ::IO::GenericStream& stream, WriteCallback callback) = 0; }; } // namespace AZ::Dom diff --git a/Code/Framework/AzCore/AzCore/azcore_files.cmake b/Code/Framework/AzCore/AzCore/azcore_files.cmake index 8557773c93..ef6c7434cc 100644 --- a/Code/Framework/AzCore/AzCore/azcore_files.cmake +++ b/Code/Framework/AzCore/AzCore/azcore_files.cmake @@ -127,6 +127,8 @@ set(FILES Debug/TraceReflection.h DOM/DomBackend.cpp DOM/DomBackend.h + DOM/DomUtils.cpp + DOM/DomUtils.h DOM/DomVisitor.cpp DOM/DomVisitor.h DOM/Backends/JSON/JsonBackend.h diff --git a/Code/Framework/AzCore/Tests/DOM/DomJsonBenchmarks.cpp b/Code/Framework/AzCore/Tests/DOM/DomJsonBenchmarks.cpp index ea72092001..0baa4aeb53 100644 --- a/Code/Framework/AzCore/Tests/DOM/DomJsonBenchmarks.cpp +++ b/Code/Framework/AzCore/Tests/DOM/DomJsonBenchmarks.cpp @@ -8,6 +8,7 @@ #if defined(HAVE_BENCHMARK) +#include #include #include #include @@ -132,7 +133,7 @@ namespace Benchmark auto result = AZ::Dom::Json::WriteToRapidJsonDocument( [&](AZ::Dom::Visitor& visitor) { - return backend.ReadFromStringInPlace(payloadCopy, visitor); + return AZ::Dom::Utils::ReadFromStringInPlace(backend, payloadCopy, visitor); }); benchmark::DoNotOptimize(result.GetValue()); @@ -152,7 +153,7 @@ namespace Benchmark auto result = AZ::Dom::Json::WriteToRapidJsonDocument( [&](AZ::Dom::Visitor& visitor) { - return backend.ReadFromString(serializedPayload, AZ::Dom::Lifetime::Temporary, visitor); + return AZ::Dom::Utils::ReadFromString(backend, serializedPayload, AZ::Dom::Lifetime::Temporary, visitor); }); benchmark::DoNotOptimize(result.GetValue()); diff --git a/Code/Framework/AzCore/Tests/DOM/DomJsonTests.cpp b/Code/Framework/AzCore/Tests/DOM/DomJsonTests.cpp index cde5e2eb3f..a416bcf86b 100644 --- a/Code/Framework/AzCore/Tests/DOM/DomJsonTests.cpp +++ b/Code/Framework/AzCore/Tests/DOM/DomJsonTests.cpp @@ -8,6 +8,7 @@ #include #include +#include #include #include #include @@ -70,7 +71,7 @@ namespace AZ::Dom::Tests { AZStd::string serializedDocument; JsonBackend backend; - auto result = backend.WriteToString(serializedDocument, visitDocumentFn); + auto result = Dom::Utils::WriteToString(backend, serializedDocument, visitDocumentFn); EXPECT_TRUE(result.IsSuccess()); EXPECT_EQ(canonicalSerializedDocument, serializedDocument); } @@ -81,7 +82,7 @@ namespace AZ::Dom::Tests [&canonicalSerializedDocument](AZ::Dom::Visitor& visitor) { JsonBackend backend; - return backend.ReadFromString(canonicalSerializedDocument, AZ::Dom::Lifetime::Temporary, visitor); + return Dom::Utils::ReadFromString(backend, canonicalSerializedDocument, Dom::Lifetime::Persistent, visitor); }); EXPECT_TRUE(result.IsSuccess()); EXPECT_EQ(AZ::JsonSerialization::Compare(*m_document, result.GetValue()), JsonSerializerCompareResult::Equal); @@ -91,11 +92,11 @@ namespace AZ::Dom::Tests { AZStd::string serializedDocument; JsonBackend backend; - auto result = backend.WriteToString( - serializedDocument, + auto result = Dom::Utils::WriteToString( + backend, serializedDocument, [&backend, &canonicalSerializedDocument](AZ::Dom::Visitor& visitor) { - return backend.ReadFromString(canonicalSerializedDocument, AZ::Dom::Lifetime::Temporary, visitor); + return Dom::Utils::ReadFromString(backend, canonicalSerializedDocument, Dom::Lifetime::Persistent, visitor); }); EXPECT_TRUE(result.IsSuccess()); EXPECT_EQ(canonicalSerializedDocument, serializedDocument);