This is an automated email from the ASF dual-hosted git repository. swebb2066 pushed a commit to branch reduce_decoding_overhead in repository https://gitbox.apache.org/repos/asf/logging-log4cxx.git
commit 40886a5098fafa03ccd62ed809a75c6830af51c2 Author: Stephen Webb <[email protected]> AuthorDate: Fri Sep 4 12:08:19 2026 +1000 Fix performance regression with JSON & XML output --- src/main/cpp/bytebuffer.cpp | 55 +++------ src/main/cpp/charsetdecoder.cpp | 28 +++-- src/main/cpp/charsetencoder.cpp | 2 +- src/main/include/log4cxx/helpers/bytebuffer.h | 3 +- src/main/include/log4cxx/private/bytebuffer_priv.h | 130 +++++++++++++++++++++ 5 files changed, 169 insertions(+), 49 deletions(-) diff --git a/src/main/cpp/bytebuffer.cpp b/src/main/cpp/bytebuffer.cpp index 62a3e770..40aa4f53 100644 --- a/src/main/cpp/bytebuffer.cpp +++ b/src/main/cpp/bytebuffer.cpp @@ -15,7 +15,7 @@ * limitations under the License. */ #include <log4cxx/logstring.h> -#include <log4cxx/helpers/bytebuffer.h> +#include <log4cxx/private/bytebuffer_priv.h> #if LOG4CXX_ABI_VERSION <= 15 #include <log4cxx/helpers/exception.h> #endif @@ -24,17 +24,6 @@ using namespace LOG4CXX_NS; using namespace LOG4CXX_NS::helpers; -struct ByteBuffer::ByteBufferPriv -{ - ByteBufferPriv(char* data1, size_t capacity) : - base(data1), pos(0), lim(capacity), cap(capacity) {} - - char* base; - size_t pos; - size_t lim; - size_t cap; -}; - ByteBuffer::ByteBuffer(char* data1, size_t capacity) : m_priv(std::make_unique<ByteBufferPriv>(data1, capacity)) { @@ -44,24 +33,24 @@ ByteBuffer::~ByteBuffer() { } +ByteBuffer::ByteBufferPriv& ByteBuffer::impl() +{ + return *m_priv; +} + void ByteBuffer::clear() { - m_priv->lim = m_priv->cap; - m_priv->pos = 0; + m_priv->clear(); } void ByteBuffer::carry() { - auto available = remaining(); - memmove(m_priv->base, current(), available); - m_priv->lim = m_priv->cap; - m_priv->pos = available; + m_priv->carry(); } void ByteBuffer::flip() { - m_priv->lim = m_priv->pos; - m_priv->pos = 0; + m_priv->flip(); } #if LOG4CXX_ABI_VERSION <= 15 @@ -95,53 +84,45 @@ void ByteBuffer::limit(size_t newLimit) bool ByteBuffer::put(char byte) { - if (m_priv->pos < m_priv->lim) - { - m_priv->base[m_priv->pos++] = byte; - return true; - } - - return false; + return m_priv->put(byte); } char* ByteBuffer::data() { - return m_priv->base; + return m_priv->data(); } const char* ByteBuffer::data() const { - return m_priv->base; + return m_priv->data(); } char* ByteBuffer::current() { - return m_priv->base + m_priv->pos; + return m_priv->current(); } const char* ByteBuffer::current() const { - return m_priv->base + m_priv->pos; + return m_priv->current(); } size_t ByteBuffer::limit() const { - return m_priv->lim; + return m_priv->limit(); } size_t ByteBuffer::position() const { - return m_priv->pos; + return m_priv->position(); } size_t ByteBuffer::remaining() const { - return m_priv->lim - m_priv->pos; + return m_priv->remaining(); } size_t ByteBuffer::increment_position(size_t byteCount) { - auto available = remaining(); - m_priv->pos += byteCount < available ? byteCount : available; - return remaining(); + return m_priv->increment_position(byteCount); } diff --git a/src/main/cpp/charsetdecoder.cpp b/src/main/cpp/charsetdecoder.cpp index fc9ce181..76ab9550 100644 --- a/src/main/cpp/charsetdecoder.cpp +++ b/src/main/cpp/charsetdecoder.cpp @@ -18,7 +18,7 @@ #include <log4cxx/private/string_c11.h> #include <log4cxx/logstring.h> #include <log4cxx/helpers/charsetdecoder.h> -#include <log4cxx/helpers/bytebuffer.h> +#include <log4cxx/private/bytebuffer_priv.h> #include <log4cxx/helpers/exception.h> #include <log4cxx/helpers/pool.h> #include <log4cxx/helpers/loglog.h> @@ -87,9 +87,10 @@ class APRCharsetDecoder : public CharsetDecoder { } - virtual log4cxx_status_t decode(ByteBuffer& in, + virtual log4cxx_status_t decode(ByteBuffer& in_param, LogString& out) { + auto& in = in_param.impl(); enum { BUFSIZE = 256 }; logchar buf[BUFSIZE]; const apr_size_t initial_outbytes_left = BUFSIZE * sizeof(logchar); @@ -167,9 +168,10 @@ class MbstowcsCharsetDecoder : public CharsetDecoder return APR_SUCCESS; } - virtual log4cxx_status_t decode(ByteBuffer& in, + virtual log4cxx_status_t decode(ByteBuffer& in_param, LogString& out) { + auto& in = in_param.impl(); log4cxx_status_t stat = APR_SUCCESS; enum { BUFSIZE = 256 }; wchar_t wbuf[BUFSIZE]; @@ -276,9 +278,10 @@ class TrivialCharsetDecoder : public CharsetDecoder { } - virtual log4cxx_status_t decode(ByteBuffer& in, + virtual log4cxx_status_t decode(ByteBuffer& in_param, LogString& out) { + auto& in = in_param.impl(); size_t remaining = in.remaining(); if ( remaining > 0) @@ -315,13 +318,14 @@ class UTF8CharsetDecoder : public CharsetDecoder } private: - virtual log4cxx_status_t decode(ByteBuffer& in, + virtual log4cxx_status_t decode(ByteBuffer& in_param, LogString& out) { + auto& in = in_param.impl(); auto availableByteCount = in.remaining(); while (0 < availableByteCount) { - auto sv = getUTF8CodePoint(in); + auto sv = getUTF8CodePoint(in_param); auto nextAvailableByteCount = in.remaining(); if (sv == 0xFFFF || nextAvailableByteCount == availableByteCount) return APR_BADCH; @@ -352,9 +356,10 @@ class ISOLatinCharsetDecoder : public CharsetDecoder } private: - virtual log4cxx_status_t decode(ByteBuffer& in, + virtual log4cxx_status_t decode(ByteBuffer& in_param, LogString& out) { + auto& in = in_param.impl(); auto availableByteCount = in.remaining(); auto src = in.current(); auto srcEnd = src + availableByteCount; @@ -394,9 +399,10 @@ class USASCIICharsetDecoder : public CharsetDecoder private: - virtual log4cxx_status_t decode(ByteBuffer& in, + virtual log4cxx_status_t decode(ByteBuffer& in_param, LogString& out) { + auto& in = in_param.impl(); log4cxx_status_t stat = APR_SUCCESS; auto availableByteCount = in.remaining(); @@ -439,8 +445,9 @@ class LocaleCharsetDecoder : public CharsetDecoder LocaleCharsetDecoder() : state() { } - log4cxx_status_t decode(ByteBuffer& in, LogString& out) override + log4cxx_status_t decode(ByteBuffer& in_param, LogString& out) override { + auto& in = in_param.impl(); log4cxx_status_t result = APR_SUCCESS; auto p = in.current(); auto availableByteCount = in.remaining(); @@ -594,8 +601,9 @@ log4cxx_status_t CharsetDecoder::decode(const char* in, size_t maxByteCount, Log return decode(buf, out); } -unsigned int CharsetDecoder::getUTF8CodePoint(ByteBuffer& in) +unsigned int CharsetDecoder::getUTF8CodePoint(ByteBuffer& in_param) { + auto& in = in_param.impl(); auto availableByteCount = in.remaining(); if (0 == availableByteCount) return 0xFFFF; diff --git a/src/main/cpp/charsetencoder.cpp b/src/main/cpp/charsetencoder.cpp index eb789a2d..e215b744 100644 --- a/src/main/cpp/charsetencoder.cpp +++ b/src/main/cpp/charsetencoder.cpp @@ -16,7 +16,7 @@ */ #include <log4cxx/logstring.h> #include <log4cxx/helpers/charsetencoder.h> -#include <log4cxx/helpers/bytebuffer.h> +#include <log4cxx/private/bytebuffer_priv.h> #include <log4cxx/helpers/exception.h> #include <apr_xlate.h> #include <log4cxx/helpers/stringhelper.h> diff --git a/src/main/include/log4cxx/helpers/bytebuffer.h b/src/main/include/log4cxx/helpers/bytebuffer.h index f2fe0c3e..e4ea054a 100644 --- a/src/main/include/log4cxx/helpers/bytebuffer.h +++ b/src/main/include/log4cxx/helpers/bytebuffer.h @@ -110,7 +110,8 @@ class LOG4CXX_EXPORT ByteBuffer /// @returns true if \c byteValue was stored in the buffer. bool put(char byteValue); - + /// Internal use only + ByteBufferPriv& impl(); private: ByteBuffer(const ByteBuffer&); ByteBuffer& operator=(const ByteBuffer&); diff --git a/src/main/include/log4cxx/private/bytebuffer_priv.h b/src/main/include/log4cxx/private/bytebuffer_priv.h new file mode 100644 index 00000000..65625417 --- /dev/null +++ b/src/main/include/log4cxx/private/bytebuffer_priv.h @@ -0,0 +1,130 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +#include <log4cxx/helpers/bytebuffer.h> + +using namespace LOG4CXX_NS; +using namespace LOG4CXX_NS::helpers; + +struct ByteBuffer::ByteBufferPriv +{ +private: // Attributes + char* base; + size_t pos; + size_t lim; + size_t cap; + +public: // ...structor + ByteBufferPriv(char* data, size_t capacity) + : base(data) + , pos(0) + , lim(capacity) + , cap(capacity) + {} + +public: // Accessors + inline char* data(); + inline const char* data() const; + inline char* current(); + inline const char* current() const; + inline size_t limit() const; + inline size_t position() const; + inline size_t remaining() const; + +public: // Modifiers + inline void carry(); + inline void clear(); + inline void flip(); + inline size_t increment_position(size_t byteCount); + inline bool put(char byteValue); + +#if LOG4CXX_ABI_VERSION <= 15 + friend class ByteBuffer; +#endif +}; + +void ByteBuffer::ByteBufferPriv::clear() +{ + this->lim = this->cap; + this->pos = 0; +} + +void ByteBuffer::ByteBufferPriv::carry() +{ + auto available = remaining(); + memmove(this->base, current(), available); + this->lim = this->cap; + this->pos = available; +} + +void ByteBuffer::ByteBufferPriv::flip() +{ + this->lim = this->pos; + this->pos = 0; +} + +bool ByteBuffer::ByteBufferPriv::put(char byte) +{ + if (this->pos < this->lim) + { + this->base[this->pos++] = byte; + return true; + } + + return false; +} + +char* ByteBuffer::ByteBufferPriv::data() +{ + return this->base; +} + +const char* ByteBuffer::ByteBufferPriv::data() const +{ + return this->base; +} + +char* ByteBuffer::ByteBufferPriv::current() +{ + return this->base + this->pos; +} + +const char* ByteBuffer::ByteBufferPriv::current() const +{ + return this->base + this->pos; +} + +size_t ByteBuffer::ByteBufferPriv::limit() const +{ + return this->lim; +} + +size_t ByteBuffer::ByteBufferPriv::position() const +{ + return this->pos; +} + +size_t ByteBuffer::ByteBufferPriv::remaining() const +{ + return this->lim - this->pos; +} + +size_t ByteBuffer::ByteBufferPriv::increment_position(size_t byteCount) +{ + auto available = remaining(); + this->pos += byteCount < available ? byteCount : available; + return remaining(); +}
