Tracktion / choc

A collection of header only classes, permissively licensed, to provide basic useful tasks with the bare-minimum of dependencies.
Other
543 stars 49 forks source link

JUCE suggestions #60

Closed Anthony-Nicholls closed 3 weeks ago

Anthony-Nicholls commented 3 weeks ago

These commits are designed to prevent JUCE needing to add any additional workarounds when including choc.

The most controversial of which I suspect will be the two changes required to make the MSVC code analyser happy but I was keen to not simply disable the warnings using pragmas, add heap allocation where there wasn't already heap allocation, or add breaking changes (which I believe I've avoided).

julianstorer commented 3 weeks ago

Thanks Anthony - those all look great, except for the thread-local change. That function needs to be fast and realtime-safe, which is why it uses a local array rather than falling back to the heap if the size is too big. I don't think it'd be safe to trust a thread-local to be lock-free (or to be fast enough). Not sure what to suggest instead.. What circumstances did you see a problem?

Anthony-Nicholls commented 3 weeks ago

I thought that might be the case.

So the warning we see is

...\choc_Value.h(2423): warning C6262: Function uses '17464' bytes of stack:  exceeds /analyze:stacksize '16384'.  Consider moving some data to heap.

This appears when code analysis is enabled on MSVC (https://learn.microsoft.com/en-us/cpp/build/reference/analyze-code-analysis). We could forcibly increase the stack size as an argument to analyse but 16384 is the default value.

Other options are

#if _MSC_VER
 #pragma warning (push)
 #pragma warning (disable: 6262)
#endif

template <typename OutputStream>
void ValueView::serialise (OutputStream& output) const

#if _MSC_VER
 #pragma warning (pop)
#endif

Alternatively we can just wrap CHOC disabling the warning.

julianstorer commented 3 weeks ago

Hmm. The number I picked is pretty arbitrary, it could be smaller.

Or.. perhaps using alloca would get around it?

Anthony-Nicholls commented 3 weeks ago

That might work! It has the nice bonus of being able to allocate only the amount requested based on the actual data size required too.

It's a little messy but this appears to be compiling on MSVC, untested otherwise though...

    ...
    check (dataSize > 0, "Invalid data size");

    uint8_t* localCopy = nullptr;

   #if _MSC_VER
    __try
    {
        localCopy = (uint8_t*) _alloca (dataSize);
    }
    __except (GetExceptionCode() == STATUS_STACK_OVERFLOW)
    {
        throwError ("Stack overflow");
    }
   #else
    localCopy = (uint8_t*) alloca (dataSize);
   #endif

    check (localCopy != nullptr, "Stack allocation failed");
    std::memcpy (localCopy, data, dataSize);
    ...