From fd4d47e4a54d815f027a636a025c1fe9f530df6a Mon Sep 17 00:00:00 2001 From: Alessio Quaglino Date: Mon, 7 Nov 2022 02:55:37 -0800 Subject: [PATCH] Format memory warning in human-friendly way. PiperOrigin-RevId: 486610408 Change-Id: I6beecb12059e69f4407e921d20efc54ce6b3f491 --- doc/APIreference.rst | 11 +++++++ include/mujoco/mujoco.h | 3 ++ introspect/functions.py | 14 +++++++++ python/mujoco/functions.cc | 1 + src/engine/engine_macro.h | 7 +++++ src/engine/engine_util_errmem.c | 6 +--- src/engine/engine_util_misc.c | 25 +++++++++++++-- src/engine/engine_util_misc.h | 3 ++ src/xml/xml_native_writer.cc | 23 +------------- test/engine/engine_util_misc_test.cc | 39 ++++++++++++++++++++++++ unity/Runtime/Bindings/MujocoBindings.cs | 4 +++ 11 files changed, 107 insertions(+), 29 deletions(-) create mode 100644 test/engine/engine_util_misc_test.cc diff --git a/doc/APIreference.rst b/doc/APIreference.rst index d1ac9b5b..1e4c4377 100644 --- a/doc/APIreference.rst +++ b/doc/APIreference.rst @@ -6601,6 +6601,17 @@ Convert type name to type id (mjtObj). .. _mju_warningText: +mju_writeNumBytes +~~~~~~~~~~~~~~~ + +.. code-block:: C + + const char* mju_writeNumBytes(const int nbytes); + +Construct a human readable number of bytes using standard letter suffix. + +.. _mju_warningText: + mju_warningText ~~~~~~~~~~~~~~~ diff --git a/include/mujoco/mujoco.h b/include/mujoco/mujoco.h index d1dd98e4..58b1631f 100644 --- a/include/mujoco/mujoco.h +++ b/include/mujoco/mujoco.h @@ -1095,6 +1095,9 @@ MJAPI const char* mju_type2Str(int type); // Convert type name to type id (mjtObj). MJAPI int mju_str2Type(const char* str); +// Return human readable number of bytes using standard letter suffix. +MJAPI const char* mju_writeNumBytes(const size_t nbytes); + // Construct a warning message given the warning type and info. MJAPI const char* mju_warningText(int warning, size_t info); diff --git a/introspect/functions.py b/introspect/functions.py index 65e14f56..20fb653f 100644 --- a/introspect/functions.py +++ b/introspect/functions.py @@ -6937,6 +6937,20 @@ FUNCTIONS: Mapping[str, FunctionDecl] = dict([ ), doc='Convert type name to type id (mjtObj).', )), + ('mju_writeNumBytes', + FunctionDecl( + name='mju_writeNumBytes', + return_type=PointerType( + inner_type=ValueType(name='char', is_const=True), + ), + parameters=( + FunctionParameterDecl( + name='nbytes', + type=ValueType(name='size_t', is_const=True), + ), + ), + doc='Return human readable number of bytes using standard letter suffix.', # pylint: disable=line-too-long + )), ('mju_warningText', FunctionDecl( name='mju_warningText', diff --git a/python/mujoco/functions.cc b/python/mujoco/functions.cc index 02580c7f..18449f14 100644 --- a/python/mujoco/functions.cc +++ b/python/mujoco/functions.cc @@ -1061,6 +1061,7 @@ PYBIND11_MODULE(_functions, pymodule) { Def(pymodule); Def(pymodule); Def(pymodule); + Def(pymodule); Def(pymodule); Def(pymodule); DEF_WITH_OMITTED_PY_ARGS(traits::mju_isZero, "n")( diff --git a/src/engine/engine_macro.h b/src/engine/engine_macro.h index 38996305..3ad6dca6 100644 --- a/src/engine/engine_macro.h +++ b/src/engine/engine_macro.h @@ -33,6 +33,13 @@ #define mjMAX(a, b) (((a) > (b)) ? (a) : (b)) #define mjMIN(a, b) (((a) < (b)) ? (a) : (b)) +// thread local macro +#ifdef _MSC_VER +#define mjTHREADLOCAL __declspec(thread) +#else +#define mjTHREADLOCAL _Thread_local +#endif + //-------------------------- timer macros ---------------------------------------------------------- diff --git a/src/engine/engine_util_errmem.c b/src/engine/engine_util_errmem.c index 1b01cf37..9ce54358 100644 --- a/src/engine/engine_util_errmem.c +++ b/src/engine/engine_util_errmem.c @@ -24,6 +24,7 @@ #include #endif +#include "engine/engine_macro.h" //------------------------- cross-platform aligned malloc/free ------------------------------------- static inline void* mju_alignedMalloc(size_t size, size_t align) { @@ -73,11 +74,6 @@ void mju_clearHandlers(void) { typedef void (*callback_fn)(const char*); -#ifdef _MSC_VER -#define mjTHREADLOCAL __declspec(thread) -#else -#define mjTHREADLOCAL _Thread_local -#endif static mjTHREADLOCAL callback_fn _mjPRIVATE_tls_error_fn = NULL; static mjTHREADLOCAL callback_fn _mjPRIVATE_tls_warning_fn = NULL; diff --git a/src/engine/engine_util_misc.c b/src/engine/engine_util_misc.c index 06f44680..bda70282 100644 --- a/src/engine/engine_util_misc.c +++ b/src/engine/engine_util_misc.c @@ -946,9 +946,30 @@ int mju_str2Type(const char* str) { +// return human readable number of bytes using standard letter suffix +const char* mju_writeNumBytes(const size_t nbytes) { + int i; + static mjTHREADLOCAL char message[20]; + static const char suffix[] = " KMGTPE"; + for (i=0; i<6; i++) { + const size_t bits = (size_t)(1) << (10*(6-i)); + if (nbytes >= bits && !(nbytes & (bits - 1))) { + break; + } + } + if (i<6) { + mjSNPRINTF(message, "%zu%c", nbytes >> (10*(6-i)), suffix[6-i]); + } else { + mjSNPRINTF(message, "%zu", nbytes >> (10*(6-i))); + } + return message; +} + + + // warning text const char* mju_warningText(int warning, size_t info) { - static char str[1000]; + static mjTHREADLOCAL char str[1000]; switch (warning) { case mjWARN_INERTIA: @@ -965,7 +986,7 @@ const char* mju_warningText(int warning, size_t info) { case mjWARN_CNSTRFULL: mjSNPRINTF(str, "Insufficient arena memory for the number of constraints generated. " - "Increase arena memory allocation above %zu bytes.", info); + "Increase arena memory allocation above %s bytes.", mju_writeNumBytes(info)); break; case mjWARN_VGEOMFULL: diff --git a/src/engine/engine_util_misc.h b/src/engine/engine_util_misc.h index 8a2b58a6..4288d380 100644 --- a/src/engine/engine_util_misc.h +++ b/src/engine/engine_util_misc.h @@ -87,6 +87,9 @@ MJAPI const char* mju_type2Str(int type); // convert type name to type id (mjtObj) MJAPI int mju_str2Type(const char* str); +// return human readable number of bytes using standard letter suffix +MJAPI const char* mju_writeNumBytes(const size_t nbytes); + // warning text MJAPI const char* mju_warningText(int warning, size_t info); diff --git a/src/xml/xml_native_writer.cc b/src/xml/xml_native_writer.cc index 2c35b11b..fc4e4531 100644 --- a/src/xml/xml_native_writer.cc +++ b/src/xml/xml_native_writer.cc @@ -830,28 +830,7 @@ void mjXWriter::Size(XMLElement* root) { // write memory if (model->memory != -1) { - const std::size_t memory = static_cast(model->memory); - const std::vector> kSuffix = { - {60, 'E'}, {50, 'P'}, {40, 'T'}, {30, 'G'}, {20, 'M'}, {10, 'K'}, - }; - - std::ostringstream strm; - - // check for divisibility by each suffixed size - for (const auto& [multiplier_bit, suffix_char] : kSuffix) { - const std::size_t multiplier = static_cast(1) << multiplier_bit; - if (memory >= multiplier && !(memory & (multiplier - 1))) { - strm << (memory >> multiplier_bit) << suffix_char; - break; - } - } - - // doesn't match any suffix, just write the number out as-is - if (!strm.tellp()) { - strm << memory; - } - - WriteAttrTxt(section, "memory", strm.str()); + WriteAttrTxt(section, "memory", mju_writeNumBytes(model->memory)); } // write sizes diff --git a/test/engine/engine_util_misc_test.cc b/test/engine/engine_util_misc_test.cc new file mode 100644 index 00000000..eb26e6ac --- /dev/null +++ b/test/engine/engine_util_misc_test.cc @@ -0,0 +1,39 @@ +// Copyright 2021 DeepMind Technologies Limited +// +// Licensed 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. + +// Tests for engine/engine_util_solve.c. + +#include +#include +#include +#include +#include "test/fixture.h" + +namespace mujoco { +namespace { + +using ::testing::HasSubstr; + +TEST_F(MujocoTest, PrintsMemoryWarning) { + EXPECT_THAT(mju_warningText(mjWARN_CNSTRFULL, pow(2, 10)), HasSubstr("1K bytes")); + EXPECT_THAT(mju_warningText(mjWARN_CNSTRFULL, pow(2, 20)), HasSubstr("1M bytes")); + EXPECT_THAT(mju_warningText(mjWARN_CNSTRFULL, pow(2, 30)), HasSubstr("1G bytes")); + EXPECT_THAT(mju_warningText(mjWARN_CNSTRFULL, pow(2, 40)), HasSubstr("1T bytes")); + EXPECT_THAT(mju_warningText(mjWARN_CNSTRFULL, pow(2, 50)), HasSubstr("1P bytes")); + EXPECT_THAT(mju_warningText(mjWARN_CNSTRFULL, pow(2, 60)), HasSubstr("1E bytes")); + EXPECT_THAT(mju_warningText(mjWARN_CNSTRFULL, pow(2, 30)+1), HasSubstr("1073741825 bytes")); +} + +} // namespace +} // namespace mujoco diff --git a/unity/Runtime/Bindings/MujocoBindings.cs b/unity/Runtime/Bindings/MujocoBindings.cs index 1ec92e7e..79d48608 100644 --- a/unity/Runtime/Bindings/MujocoBindings.cs +++ b/unity/Runtime/Bindings/MujocoBindings.cs @@ -3516,6 +3516,10 @@ public static unsafe extern string mju_type2Str(int type); [DllImport("mujoco", CallingConvention = CallingConvention.Cdecl)] public static unsafe extern int mju_str2Type([MarshalAs(UnmanagedType.LPStr)]string str); +[DllImport("mujoco", CallingConvention = CallingConvention.Cdecl)] +[return: MarshalAs(UnmanagedType.LPStr)] +public static unsafe extern string mju_writeNumBytes(UIntPtr nbytes); + [DllImport("mujoco", CallingConvention = CallingConvention.Cdecl)] [return: MarshalAs(UnmanagedType.LPStr)] public static unsafe extern string mju_warningText(int warning, UIntPtr info);