Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for OS trace events #20

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

thirtytwobits
Copy link

Many operating systems support debugging via "trace" events which are high performance function calls to a tracing engine that collects this data for later analysis. This change allows o1heap to provide heap allocation trace events when O1HEAP_TRACE is defined.

Copy link
Owner

@pavel-kirienko pavel-kirienko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CI broke because of the outdated Sonar script; I should fix that separately.

o1heap/o1heap.h Outdated
Comment on lines 36 to 59
/// Define O1HEAP_TRACE to enable two extern symbols your application can implement when integrating with trace
/// tooling:
///
/// Invoked from o1heapAllocate with a pointer to the newly allocated memory and the size of the
/// memory region. Note that size_bytes is not the size of the memory requested but the size of
/// the memory fragment reserved.
///
/// if allocated_memory is NULL an allocation failure has occurred. In this case size_bytes is the
/// number of bytes requested that the allocator could not provide.
///
/// void o1heapAllocateTrace(O1HeapInstance* const handle, void* const allocated_memory, const size_t size_bytes);
///
///
/// Invoked from o1heapFree with a pointer to the previously allocated memory and the size of the
/// memory region released. Note that size_bytes is not the size of the memory originally requested
/// but the size of the memory fragment that was released.
///
/// freed_memory must not be accessed. It is provided for it's address only.
///
/// void o1heapFreeTrace(O1HeapInstance* const handle, void* const freed_memory, size_t size_bytes);
///
/// If you define this macro you _must_ implement these methods or the link stage of your build will fail.
///
/// #define O1HEAP_TRACE
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Build-time configuration options go into o1heap.c. There is a section with various options in the beginning, it is also mentioned in the README. The reason I don't want them in the header file because I don't want to contaminate the scope of the header file with definitions that are not directly related to the runtime API.

o1heap/o1heap.c Outdated
@@ -401,6 +420,10 @@ void o1heapFree(O1HeapInstance* const handle, void* const pointer)
O1HEAP_ASSERT(frag->header.size <= handle->diagnostics.capacity);
O1HEAP_ASSERT((frag->header.size % FRAGMENT_SIZE_MIN) == 0U);

#ifdef O1HEAP_TRACE
o1heapFreeTrace(handle, pointer, frag->header.size);
#endif
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Preprocessor conditionals could be avoided if we replaced the functions with:

#ifndef O1HEAP_TRACE_ALLOCATE
#   define O1HEAP_TRACE_ALLOCATE(handle, pointer, size)
#endif
#ifndef O1HEAP_TRACE_FREE
#   define O1HEAP_TRACE_FREE(handle, pointer, size)
#endif

Then the user would provide the required callbacks via O1HEAP_CONFIG_HEADER, similar to some other C libraries (LittleFS or Unity for example):

// My custom config header: o1heap_config.h
// Add -DO1HEAP_CONFIG_HEADER=o1heap_config.h to the build flags

extern void o1heapAllocateTrace(O1HeapInstance* const handle, void* const allocated_memory, size_t size_bytes);
extern void o1heapFreeTrace(O1HeapInstance* const handle, void* const freed_memory, size_t size_bytes);

#define O1HEAP_TRACE_ALLOCATE(handle, pointer, size) \
    o1heapAllocateTrace(handle, pointer, trace)
#define O1HEAP_TRACE_FREE(handle, pointer, size) \
    o1heapFreeTrace(handle, pointer, size)

Why this is better:

  • The code is not contaminated with conditional preprocessor blocks.
  • Fewer API entities are introduced.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pattern seems a bit more stable and familiar:

#ifdef O1HEAP_INCLUDE_CONFIG_HEADER
#include "o1heap_config.h"
#endif

I'm not even sure you can set an include name from a macro?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ofc
image

Many operating systems support debugging via "trace" events which are high performance function calls to a tracing engine that collects this data for later analysis. This change allows o1heap to provide heap allocation trace events when O1HEAP_TRACE is defined.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants