From fbff29957cd165e6262359629a672e240d45c3bb Mon Sep 17 00:00:00 2001 From: David Steele Date: Thu, 18 Jun 2020 13:13:55 -0400 Subject: [PATCH] Inline strPtr() to increase profiling accuracy. strPtr() is called more than any other function and during profiling (with or without optimization) it can end up using a disproportionate amount of the total runtime. Even though it is fast, the profiler has a minimum resolution for each function call so strPtr() will often end up towards the top of the list even though the real runtime is quite small. Instead, inline strPtr() and indicate to gcc that it should be inlined even for non-optimized builds, since that's how profiles are usually generated. To make strPtr() smaller require "this" to be non-NULL and add another function, strPtrNull(), to deal with the few cases where we need NULL handling. As a bonus this makes the executable about 1% smaller even when compared to a prior optimized build which would inline some percentage of strPtr() calls. --- doc/xml/release.xml | 8 +++++ src/common/io/tls/client.c | 2 +- src/common/type/string.c | 2 +- src/common/type/string.h | 48 +++++++++++++++---------- test/src/common/harnessTest.h | 8 ++--- test/src/module/common/typeStringTest.c | 6 ++-- 6 files changed, 47 insertions(+), 27 deletions(-) diff --git a/doc/xml/release.xml b/doc/xml/release.xml index 5dbd8fdb1..677aafaa3 100644 --- a/doc/xml/release.xml +++ b/doc/xml/release.xml @@ -68,6 +68,14 @@

Improve behavior of the repo-ls command.

+ + + + + + +

Inline strPtr() to increase profiling accuracy.

+ diff --git a/src/common/io/tls/client.c b/src/common/io/tls/client.c index e03f37100..f5c8894ef 100644 --- a/src/common/io/tls/client.c +++ b/src/common/io/tls/client.c @@ -104,7 +104,7 @@ tlsClientNew(SocketClient *socket, TimeMSec timeout, bool verifyPeer, const Stri if (caFile != NULL || caPath != NULL) // {vm_covered} { cryptoError( // {vm_covered} - SSL_CTX_load_verify_locations(this->context, strPtr(caFile), strPtr(caPath)) != 1, // {vm_covered} + SSL_CTX_load_verify_locations(this->context, strPtrNull(caFile), strPtrNull(caPath)) != 1, // {vm_covered} "unable to set user-defined CA certificate location"); // {vm_covered} } // Else use the defaults diff --git a/src/common/type/string.c b/src/common/type/string.c index 77ef1eb05..395be89ad 100644 --- a/src/common/type/string.c +++ b/src/common/type/string.c @@ -695,7 +695,7 @@ strPathAbsolute(const String *this, const String *base) /**********************************************************************************************************************************/ const char * -strPtr(const String *this) +strPtrNull(const String *this) { FUNCTION_TEST_BEGIN(); FUNCTION_TEST_PARAM(STRING, this); diff --git a/src/common/type/string.h b/src/common/type/string.h index 935cedf70..af7d2cb0d 100644 --- a/src/common/type/string.h +++ b/src/common/type/string.h @@ -35,8 +35,25 @@ String object ***********************************************************************************************************************************/ typedef struct String String; +#include "common/assert.h" #include "common/type/buffer.h" +/*********************************************************************************************************************************** +Fields that are common between dynamically allocated and constant strings + +There is nothing user-accessible here but this construct allows constant strings to be created and then handled by the same +functions that process dynamically allocated strings. +***********************************************************************************************************************************/ +#define STRING_COMMON \ + uint64_t size:32; /* Actual size of the string */ \ + uint64_t extra:32; /* Extra space allocated for expansion */ \ + char *buffer; /* String buffer */ + +typedef struct StringConst +{ + STRING_COMMON +} StringConst; + /*********************************************************************************************************************************** Constructors ***********************************************************************************************************************************/ @@ -116,8 +133,19 @@ String *strPath(const String *this); // Combine with a base path to get an absolute path String *strPathAbsolute(const String *this, const String *base); -// String pointer -const char *strPtr(const String *this); +// Pointer to zero-terminated string. strPtrNull() returns NULL when the String is NULL. +__attribute__((always_inline)) static inline const char * +strPtr(const String *this) +{ + // Avoid uncovered branch during coverage testing +#ifndef DEBUG_COVERAGE + ASSERT(this != NULL); +#endif + + return ((const StringConst *)this)->buffer; +} + +const char *strPtrNull(const String *this); // Quote a string String *strQuote(const String *this, const String *quote); @@ -149,22 +177,6 @@ Destructor ***********************************************************************************************************************************/ void strFree(String *this); -/*********************************************************************************************************************************** -Fields that are common between dynamically allocated and constant strings - -There is nothing user-accessible here but this construct allows constant strings to be created and then handled by the same -functions that process dynamically allocated strings. -***********************************************************************************************************************************/ -#define STRING_COMMON \ - uint64_t size:32; /* Actual size of the string */ \ - uint64_t extra:32; /* Extra space allocated for expansion */ \ - char *buffer; /* String buffer */ - -typedef struct StringConst -{ - STRING_COMMON -} StringConst; - /*********************************************************************************************************************************** Macros for constant strings diff --git a/test/src/common/harnessTest.h b/test/src/common/harnessTest.h index b7e21db8d..8aba54ce5 100644 --- a/test/src/common/harnessTest.h +++ b/test/src/common/harnessTest.h @@ -239,13 +239,13 @@ Macros to compare results of common data types TEST_RESULT_Z_PARAM(statement, expected, harnessTestResultOperationNe, __VA_ARGS__); #define TEST_RESULT_STR(statement, resultExpected, ...) \ - TEST_RESULT_Z(strPtr(statement), strPtr(resultExpected), __VA_ARGS__); + TEST_RESULT_Z(strPtrNull(statement), strPtrNull(resultExpected), __VA_ARGS__); #define TEST_RESULT_STR_Z(statement, resultExpected, ...) \ - TEST_RESULT_Z(strPtr(statement), resultExpected, __VA_ARGS__); + TEST_RESULT_Z(strPtrNull(statement), resultExpected, __VA_ARGS__); #define TEST_RESULT_STR_Z_KEYRPL(statement, resultExpected, ...) \ - TEST_RESULT_Z(strPtr(statement), hrnReplaceKey(resultExpected), __VA_ARGS__); + TEST_RESULT_Z(strPtrNull(statement), hrnReplaceKey(resultExpected), __VA_ARGS__); #define TEST_RESULT_Z_STR(statement, resultExpected, ...) \ - TEST_RESULT_Z(statement, strPtr(resultExpected), __VA_ARGS__); + TEST_RESULT_Z(statement, strPtrNull(resultExpected), __VA_ARGS__); #define TEST_RESULT_UINT_PARAM(statement, expected, operation, ...) \ do \ diff --git a/test/src/module/common/typeStringTest.c b/test/src/module/common/typeStringTest.c index 7b8541f65..8d2732ede 100644 --- a/test/src/module/common/typeStringTest.c +++ b/test/src/module/common/typeStringTest.c @@ -15,7 +15,7 @@ testRun(void) FUNCTION_HARNESS_VOID(); // ***************************************************************************************************************************** - if (testBegin("strNew(), strNewBuf(), strNewN(), strEmpty(), strPtr(), strSize(), and strFree()")) + if (testBegin("strNew(), strNewBuf(), strNewN(), strEmpty(), strPtr(), strPtrNull(), strSize(), and strFree()")) { // We don't want this struct to grow since there are generally a lot of strings, so make sure it doesn't grow without us // knowing about it @@ -30,7 +30,7 @@ testRun(void) TEST_RESULT_UINT(strSize(string), 13, "check size"); TEST_RESULT_BOOL(strEmpty(string), false, "is not empty"); TEST_RESULT_UINT(strlen(strPtr(string)), 13, "check size with strlen()"); - TEST_RESULT_INT(strPtr(string)[2], 'a', "check character"); + TEST_RESULT_INT(strPtrNull(string)[2], 'a', "check character"); TEST_RESULT_VOID(strFree(string), "free string"); @@ -47,7 +47,7 @@ testRun(void) // ------------------------------------------------------------------------------------------------------------------------- string = strNewFmt("formatted %s %04d", "string", 1); TEST_RESULT_STR_Z(string, "formatted string 0001", "new with formatted string"); - TEST_RESULT_STR(NULL, NULL, "null string pointer"); + TEST_RESULT_Z(strPtrNull(NULL), NULL, "null string pointer"); TEST_RESULT_VOID(strFree(string), "free string"); TEST_RESULT_VOID(strFree(NULL), "free null string");