From 93c4c9cb6a4be3b612b285d88574cf47f33ca3f8 Mon Sep 17 00:00:00 2001 From: Jian Chen Date: Mon, 9 Sep 2024 13:20:17 -0700 Subject: [PATCH] Using wostringstream only on Windows (#21938) ### Description Using wostringstream only on Windows ### Motivation and Context From line [62](https://github.com/microsoft/onnxruntime/pull/21938/files#diff-47776d020ac08134de4059eab473550237f4999c598ab56afad3676d2f193edcR62), currently, `stream_` can be either `wostringstream` or `ostringstream` depending on the OS, however, for Unix like system, `stream_` should be `ostringstream`, instead of. --- .../core/common/logging/sinks/file_sink.h | 43 +++++++++++++++++++ .../core/common/logging/sinks/ostream_sink.cc | 3 +- .../core/common/logging/sinks/ostream_sink.h | 3 +- .../test/common/logging/logging_test.cc | 17 +++++++- onnxruntime/test/common/logging/sinks_test.cc | 8 +++- 5 files changed, 69 insertions(+), 5 deletions(-) diff --git a/onnxruntime/core/common/logging/sinks/file_sink.h b/onnxruntime/core/common/logging/sinks/file_sink.h index ba3ff3e0b3..2b151a907a 100644 --- a/onnxruntime/core/common/logging/sinks/file_sink.h +++ b/onnxruntime/core/common/logging/sinks/file_sink.h @@ -8,6 +8,7 @@ namespace onnxruntime { namespace logging { +#ifndef _WIN32 /// /// ISink that writes to a file. /// @@ -47,5 +48,47 @@ class FileSink : public OStreamSink { std::unique_ptr file_; bool filter_user_data_; }; + +#else +/// +/// ISink that writes to a file. +/// +/// +class FileSink : public WOStreamSink { + public: + /// + /// Initializes a new instance of the class. + /// + /// The filename to write to. + /// If set to true [append to file]. Otherwise truncate. + /// If set to true [removes user data]. + /// Filtering of user data can alternatively be done at the level. + FileSink(std::unique_ptr file, bool filter_user_data) + : WOStreamSink(*file, /*flush*/ true), file_(std::move(file)), filter_user_data_{filter_user_data} { + } + + /// + /// Initializes a new instance of the class. + /// + /// The filename to write to. + /// If set to true [append to file]. Otherwise truncate. + /// If set to true [removes user data]. + /// Filtering of user data can alternatively be done at the level. + FileSink(const std::wstring& filename, bool append, bool filter_user_data) + : FileSink{std::make_unique(filename, std::ios::out | (append ? std::ios::app : std::ios::trunc)), + filter_user_data} { + } + + private: + void SendImpl(const Timestamp& timestamp, const std::string& logger_id, const Capture& message) override { + if (!filter_user_data_ || message.DataType() != DataType::USER) { + WOStreamSink::SendImpl(timestamp, logger_id, message); + } + } + + std::unique_ptr file_; + bool filter_user_data_; +}; +#endif } // namespace logging } // namespace onnxruntime diff --git a/onnxruntime/core/common/logging/sinks/ostream_sink.cc b/onnxruntime/core/common/logging/sinks/ostream_sink.cc index a120138d1d..033f4d2573 100644 --- a/onnxruntime/core/common/logging/sinks/ostream_sink.cc +++ b/onnxruntime/core/common/logging/sinks/ostream_sink.cc @@ -21,6 +21,7 @@ struct Color { }; #endif +#ifndef _WIN32 void OStreamSink::SendImpl(const Timestamp& timestamp, const std::string& logger_id, const Capture& message) { // operator for formatting of timestamp in ISO8601 format including microseconds using timestamp_ns::operator<<; @@ -62,7 +63,7 @@ void OStreamSink::SendImpl(const Timestamp& timestamp, const std::string& logger stream_->flush(); } } -#ifdef _WIN32 +#else void WOStreamSink::SendImpl(const Timestamp& timestamp, const std::string& logger_id, const Capture& message) { // operator for formatting of timestamp in ISO8601 format including microseconds using date::operator<<; diff --git a/onnxruntime/core/common/logging/sinks/ostream_sink.h b/onnxruntime/core/common/logging/sinks/ostream_sink.h index 1d0d24b51d..2e76243180 100644 --- a/onnxruntime/core/common/logging/sinks/ostream_sink.h +++ b/onnxruntime/core/common/logging/sinks/ostream_sink.h @@ -12,6 +12,7 @@ namespace onnxruntime { namespace logging { +#ifndef _WIN32 /// /// A std::ostream based ISink /// @@ -29,7 +30,7 @@ class OStreamSink : public ISink { std::ostream* stream_; const bool flush_; }; -#ifdef _WIN32 +#else /// /// A std::wostream based ISink /// diff --git a/onnxruntime/test/common/logging/logging_test.cc b/onnxruntime/test/common/logging/logging_test.cc index a4e843c855..d3af022f83 100644 --- a/onnxruntime/test/common/logging/logging_test.cc +++ b/onnxruntime/test/common/logging/logging_test.cc @@ -240,18 +240,30 @@ TEST_F(LoggingTestsFixture, TestVLog) { #endif } +#ifdef _WIN32 +class CTestSink : public WOStreamSink { + public: + CTestSink(std::wostringstream& stream) : WOStreamSink(stream, /*flush*/ true) { + } +}; +#else class CTestSink : public OStreamSink { public: CTestSink(std::ostringstream& stream) : OStreamSink(stream, /*flush*/ true) { } }; +#endif TEST_F(LoggingTestsFixture, TestTruncation) { const std::string logger_id{"TestTruncation"}; const Severity min_log_level = Severity::kVERBOSE; constexpr bool filter_user_data = false; +#ifdef _WIN32 + std::wostringstream out; +#else std::ostringstream out; +#endif auto* sink_ptr = new CTestSink{out}; LoggingManager manager{std::unique_ptr(sink_ptr), min_log_level, filter_user_data, @@ -261,8 +273,11 @@ TEST_F(LoggingTestsFixture, TestTruncation) { // attempt to print string longer than hard-coded 2K buffer limit LOGF(*logger, ERROR, "%s", std::string(4096, 'a').c_str()); - +#ifdef _WIN32 + EXPECT_THAT(out.str(), HasSubstr(L"[...truncated...]")); +#else EXPECT_THAT(out.str(), HasSubstr("[...truncated...]")); +#endif } TEST_F(LoggingTestsFixture, TestStreamMacroFromConditionalWithoutCompoundStatement) { diff --git a/onnxruntime/test/common/logging/sinks_test.cc b/onnxruntime/test/common/logging/sinks_test.cc index ea6c34d022..ab28940f4f 100644 --- a/onnxruntime/test/common/logging/sinks_test.cc +++ b/onnxruntime/test/common/logging/sinks_test.cc @@ -1,6 +1,6 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. - +#include "core/common/common.h" #include "core/common/logging/capture.h" #include "core/common/logging/logging.h" #include "core/common/logging/sinks/cerr_sink.h" @@ -126,9 +126,13 @@ TEST(LoggingTests, TestFileSink) { // create scoped manager so sink gets destroyed once done { +#ifdef _WIN32 + LoggingManager manager{std::make_unique(onnxruntime::ToWideString(filename), false, false), + min_log_level, false, InstanceType::Temporal}; +#else LoggingManager manager{std::unique_ptr{new FileSink{filename, false, false}}, min_log_level, false, InstanceType::Temporal}; - +#endif auto logger = manager.CreateLogger(logid); LOGS(*logger, WARNING) << message;