Skip to content

Commit

Permalink
Merge pull request #723 from bugsnag/improve-thread-perf
Browse files Browse the repository at this point in the history 
Improve performance of thread capture
  • Loading branch information
@fractalwrench
fractalwrench committed Jun 30, 2020
2 parents 551da1b + ec806b0 commit d951100
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 53 deletions.
5 changes: 3 additions & 2 deletions Bugsnag/KSCrash/Source/KSCrash/Recording/BSG_KSCrash.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -316,8 +316,9 @@ - (void)sendAllReports {
char *trace = bsg_kscrash_captureThreadTrace(depth, numFrames, callstack);
free(callstack);
NSDictionary *json = BSGDeserializeJson(trace);

if (json) {
free(trace);

if (json) {
return [BugsnagThread threadsFromArray:[json valueForKeyPath:@"crash.threads"]
binaryImages:json[@"binary_images"]
depth:depth
Expand Down
8 changes: 3 additions & 5 deletions Bugsnag/KSCrash/Source/KSCrash/Recording/BSG_KSCrashC.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -225,8 +225,9 @@ void bsg_kscrash_setThreadTracingEnabled(int threadTracingEnabled) {
}

char *bsg_kscrash_captureThreadTrace(int discardDepth, int frameCount, uintptr_t *callstack) {
bsg_kscrashsentry_suspend_threads_user();
BSG_KSCrash_Context *context = crashContext();
// capture all threads only when set to BSGThreadSendPolicyAlways
bool captureAllThreads = context->crash.threadTracingEnabled == 0;

// populate context with pre-recorded stacktrace/thread info
// for KSCrash to serialize
Expand All @@ -236,8 +237,5 @@ char *bsg_kscrash_captureThreadTrace(int discardDepth, int frameCount, uintptr_t
context->crash.offendingThread = bsg_ksmachthread_self();
context->crash.crashType = BSG_KSCrashTypeUserReported;
context->crash.suspendThreadsForUserReported = true;

char *trace = bsg_kscrw_i_captureThreadTrace(context);
bsg_kscrashsentry_resume_threads_user(false);
return trace;
return bsg_kscrw_i_captureThreadTrace(context, captureAllThreads);
}
90 changes: 45 additions & 45 deletions Bugsnag/KSCrash/Source/KSCrash/Recording/BSG_KSCrashReport.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,18 @@ typedef ucontext_t SignalUserContext;
/** The minimum length for a valid string. */
#define BSG_kMinStringLength 4

// ============================================================================
#pragma mark - Thread Data Buffer -
// ============================================================================

#define BSG_THREAD_DATA_SIZE_INITIAL 128 * 1024
#define BSG_THREAD_DATA_SIZE_INCREMENT 8 * 1024

typedef struct {
char *data;
size_t allocated_size;
} BSG_ThreadDataBuffer;

// ============================================================================
#pragma mark - Formatting -
// ============================================================================
Expand Down Expand Up @@ -615,7 +627,9 @@ void bsg_kscrw_i_writeMemoryContents(
const BSG_KSCrashReportWriter *const writer, const char *const key,
const uintptr_t address, int *limit);

void bsg_kscrw_i_writeTraceInfo(const BSG_KSCrash_Context *crashContext, const BSG_KSCrashReportWriter *writer);
void bsg_kscrw_i_writeTraceInfo(const BSG_KSCrash_Context *crashContext,
const BSG_KSCrashReportWriter *writer,
const bool captureAllThreads);

bool bsg_kscrw_i_exceedsBufferLen(const size_t length);

Expand Down Expand Up @@ -971,13 +985,8 @@ void bsg_kscrw_i_writeThread(const BSG_KSCrashReportWriter *const writer,
const char *const key,
const BSG_KSCrash_SentryContext *const crash,
const thread_t thread, const int index,
const bool writeNotableAddresses,
const bool recordAllThreads) {
const bool writeNotableAddresses) {
bool isCrashedThread = thread == crash->offendingThread;
if (!isCrashedThread && !recordAllThreads) {
return;
}

BSG_STRUCT_MCONTEXT_L machineContextBuffer;
uintptr_t backtraceBuffer[BSG_kMaxBacktraceDepth];
int backtraceLength = sizeof(backtraceBuffer) / sizeof(*backtraceBuffer);
Expand Down Expand Up @@ -1026,7 +1035,7 @@ void bsg_kscrw_i_writeThread(const BSG_KSCrashReportWriter *const writer,
* @param writeNotableAddresses whether notable addresses should be written
* so additional information about the error can be extracted
* @param recordAllThreads controls whether all threads are captured. If this parameter is false,
* the threads are still suspended but only the main thread's stacktrace is serialized.
* only the main thread's stacktrace is serialized.
*/
void bsg_kscrw_i_writeAllThreads(const BSG_KSCrashReportWriter *const writer,
const char *const key,
Expand All @@ -1047,8 +1056,10 @@ void bsg_kscrw_i_writeAllThreads(const BSG_KSCrashReportWriter *const writer,
writer->beginArray(writer, key);
{
for (mach_msg_type_number_t i = 0; i < numThreads; i++) {
bsg_kscrw_i_writeThread(writer, NULL, crash, threads[i], (int)i,
writeNotableAddresses, recordAllThreads);
thread_t thread = threads[i];
if (recordAllThreads || thread == crash->offendingThread) {
bsg_kscrw_i_writeThread(writer, NULL, crash, thread, (int) i, writeNotableAddresses);
}
}
}
writer->endContainer(writer);
Expand Down Expand Up @@ -1511,7 +1522,7 @@ void bsg_kscrashreport_writeMinimalReport(
writer, BSG_KSCrashField_CrashedThread, &crashContext->crash,
crashContext->crash.offendingThread,
bsg_kscrw_i_threadIndex(crashContext->crash.offendingThread),
false, false);
false);
bsg_kscrw_i_writeError(writer, BSG_KSCrashField_Error,
&crashContext->crash);
}
Expand Down Expand Up @@ -1638,7 +1649,7 @@ void bsg_kscrashreport_writeKSCrashFields(BSG_KSCrash_Context *crashContext, BSG
bsg_kscrw_i_addJSONElement(writer, BSG_KSCrashField_User,
crashContext->config.userInfoJSON);
}
bsg_kscrw_i_writeTraceInfo(crashContext, writer);
bsg_kscrw_i_writeTraceInfo(crashContext, writer, crashContext->crash.threadTracingEnabled == 0);
}

void bsg_kscrashreport_logCrash(const BSG_KSCrash_Context *const crashContext) {
Expand All @@ -1647,57 +1658,50 @@ void bsg_kscrashreport_logCrash(const BSG_KSCrash_Context *const crashContext) {
bsg_kscrw_i_logCrashThreadBacktrace(&crashContext->crash);
}

static const size_t bsg_g_buffer_increment = sizeof(char) * 8 * 1024;
static size_t bsg_g_thread_json_size = sizeof(char) * 128 * 1024;
static char *bsg_g_thread_json_data = NULL;

int bsg_kscrw_i_collectJsonData(const char *const data, const size_t length, void *const userData) {
if (bsg_g_thread_json_data == NULL) {
BSG_ThreadDataBuffer *thread_data = (BSG_ThreadDataBuffer *)userData;
if (thread_data->data == NULL) {
// Allocate initial memory for JSON data
void *ptr = malloc(bsg_g_thread_json_size);
void *ptr = malloc(BSG_THREAD_DATA_SIZE_INITIAL);
if (ptr != NULL) {
bsg_g_thread_json_data = ptr;
*bsg_g_thread_json_data = '\0';
} else { // failed to allocate enough memory
thread_data->data = ptr;
*thread_data->data = '\0';
thread_data->allocated_size = BSG_THREAD_DATA_SIZE_INITIAL;
} else { // failed to allocate enough memory - abandon collection
return BSG_KSJSON_ERROR_CANNOT_ADD_DATA;
}
}
if (strlen(bsg_g_thread_json_data) + length >= bsg_g_thread_json_size) {
while (strlen(thread_data->data) + length >= thread_data->allocated_size) {
// Expand memory to hold further data
bsg_g_thread_json_size += bsg_g_buffer_increment;
void *ptr = realloc(bsg_g_thread_json_data, bsg_g_thread_json_size);
void *ptr = realloc(thread_data->data, thread_data->allocated_size + BSG_THREAD_DATA_SIZE_INCREMENT);
if (ptr != NULL) {
bsg_g_thread_json_data = ptr;
} else { // failed to allocate enough memory
free(bsg_g_thread_json_data);
thread_data->data = ptr;
thread_data->allocated_size += BSG_THREAD_DATA_SIZE_INCREMENT;
} else { // failed to allocate enough memory - abandon collection
return BSG_KSJSON_ERROR_CANNOT_ADD_DATA;
}
}
strncat(bsg_g_thread_json_data, data, length);
strncat(thread_data->data, data, length);
return BSG_KSJSON_OK;
}

void bsg_kscrw_i_resetThreadTraceData() {
if (bsg_g_thread_json_data != NULL) {
*bsg_g_thread_json_data = '\0';
}
}

char *bsg_kscrw_i_captureThreadTrace(const BSG_KSCrash_Context *crashContext) {
char *bsg_kscrw_i_captureThreadTrace(const BSG_KSCrash_Context *crashContext, const bool captureAllThreads) {
BSG_KSJSONEncodeContext jsonContext;
BSG_KSCrashReportWriter concreteWriter;
BSG_KSCrashReportWriter *writer = &concreteWriter;
bsg_kscrw_i_prepareReportWriter(writer, &jsonContext);
bsg_kscrw_i_resetThreadTraceData();
bsg_ksjsonbeginEncode(bsg_getJsonContext(writer), false, bsg_kscrw_i_collectJsonData, 0);
BSG_ThreadDataBuffer userData = { NULL, 0 };
bsg_ksjsonbeginEncode(bsg_getJsonContext(writer), false, bsg_kscrw_i_collectJsonData, &userData);
writer->beginObject(writer, BSG_KSCrashField_Report);
bsg_kscrw_i_writeTraceInfo(crashContext, writer);
bsg_kscrw_i_writeTraceInfo(crashContext, writer, captureAllThreads);
writer->endContainer(writer);
bsg_ksjsonendEncode(bsg_getJsonContext(writer));
return bsg_g_thread_json_data;
return userData.data;
}

void bsg_kscrw_i_writeTraceInfo(const BSG_KSCrash_Context *crashContext, const BSG_KSCrashReportWriter *writer) {
void bsg_kscrw_i_writeTraceInfo(const BSG_KSCrash_Context *crashContext,
const BSG_KSCrashReportWriter *writer,
const bool captureAllThreads) {
bool unhandledCrash = crashContext->crash.crashType != BSG_KSCrashTypeUserReported;

// Don't write the binary images for user reported crashes to improve performance
Expand All @@ -1706,12 +1710,8 @@ void bsg_kscrw_i_writeTraceInfo(const BSG_KSCrash_Context *crashContext, const B
}
writer->beginObject(writer, BSG_KSCrashField_Crash);
{
// Conditionally write threads depending on user configuration
int sendPolicy = crashContext->crash.threadTracingEnabled;
bool recordAllThreads = sendPolicy == 0 || (unhandledCrash && sendPolicy == 1);

bsg_kscrw_i_writeAllThreads(writer, BSG_KSCrashField_Threads, &crashContext->crash,
crashContext->config.introspectionRules.enabled, recordAllThreads);
crashContext->config.introspectionRules.enabled, captureAllThreads);
bsg_kscrw_i_writeError(writer, BSG_KSCrashField_Error,&crashContext->crash);
}
writer->endContainer(writer);
Expand Down
3 changes: 2 additions & 1 deletion Bugsnag/KSCrash/Source/KSCrash/Recording/BSG_KSCrashReport.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,11 @@ void bsg_kscrashreport_logCrash(const BSG_KSCrash_Context *const crashContext);
*
* @param crashContext Contextual information about the crash and environment.
* The caller must fill this out before passing it in.
* @param captureAllThreads whether all threads should be captured, or just the current callstack
*
* @return the thread trace encoded as a JSON string
*/
char *bsg_kscrw_i_captureThreadTrace(const BSG_KSCrash_Context *crashContext);
char *bsg_kscrw_i_captureThreadTrace(const BSG_KSCrash_Context *crashContext, const bool captureAllThreads);

#ifdef __cplusplus
}
Expand Down

0 comments on commit d951100

Please sign in to comment.