1
0
mirror of https://github.com/pgbackrest/pgbackrest.git synced 2025-01-18 04:58:51 +02:00

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.
This commit is contained in:
David Steele 2020-06-18 13:13:55 -04:00 committed by GitHub
parent 3d74ec1190
commit fbff29957c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 47 additions and 27 deletions

View File

@ -68,6 +68,14 @@
<p>Improve behavior of the <cmd>repo-ls</cmd> command.</p>
</release-item>
<release-item>
<release-item-contributor-list>
<release-item-reviewer id="cynthia.shang"/>
</release-item-contributor-list>
<p>Inline <code>strPtr()</code> to increase profiling accuracy.</p>
</release-item>
</release-development-list>
</release-core-list>

View File

@ -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

View File

@ -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);

View File

@ -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

View File

@ -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 \

View File

@ -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");