Elide memtrace when disabled #97
Conversation
This resolves the memory tracer reporting calls to deallocate unmanaged memory
There was a problem hiding this comment.
Hey, I'd welcome this change and the tests pass, as the github action shows. The example app doesn't seem to compile tho.
CC: ../../src/playdate/api.nim
CC: playdate_example.nim
/Users/ninovanhooff/PlaydateProjects/playdate-nim/src/playdate/api.nim:20:135: error: incompatible function pointer types passing 'void (*const)(const char *, ...)' to parameter of type 'tyProc__YfDRPZutFzFrjPOw19ct0Pg' (aka 'void (*)(char *, ...)') [-Wincompatible-function-pointer-types]
20 | nimln_(20); _ZN8initreqs11initPrereqsE4procI7pointer7pointer4uIntE4procI7cstringE((*(*playdateAPIX60gensym42__p0).system).realloc, (*(*playdateAPIX60gensym42__p0).system).logToConsole);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/ninovanhooff/.cache/nim/playdate_example_d/simulator/@mplaydate_example.nim.c:515:177: note: passing argument to parameter 'log_p1' here
515 | N_LIB_PRIVATE N_NIMCALL(void, _ZN8initreqs11initPrereqsE4procI7pointer7pointer4uIntE4procI7cstringE)(tyProc__K8zkGJyGTJQIxluyMf4zXw realloc_p0, tyProc__YfDRPZutFzFrjPOw19ct0Pg log_p1);
| ^
1 error generated.
Error: execution of an external compiler program 'clang -c -w -ferror-limit=3 -DTARGET_EXTENSION=1 -Wall -Wno-unknown-pragmas -Wdouble-promotion -I/Users/ninovanhooff/Developer/PlaydateSDK/C_API -arch x86_64 -arch arm64 -DTARGET_SIMULATOR=1 -Wstrict-prototypes -g -I/Users/ninovanhooff/.choosenim/toolchains/nim-2.0.10/lib -I/Users/ninovanhooff/PlaydateProjects/playdate-nim/playdate_example/src -o /Users/ninovanhooff/.cache/nim/playdate_example_d/simulator/@mplaydate_example.nim.c.o /Users/ninovanhooff/.cache/nim/playdate_example_d/simulator/@mplaydate_example.nim.c' failed with exit code: 1
Perhaps there is some confusion regarding logToConsole from the bindings/system module and the exposed system module?
|
Let's see if #90 is fixed with this change |
|
Oof. This looks like a Nim compiler bug related to compatibility with GCC 14. You can find a similar issue here: nim-lang/Nim#23665 |
|
I’ve got a fix for this, I think. Let me do some testing and I’ll push |
This was causing an error in GCC 14 related to incompatible pointer types -- 'void (*)(char *, ...)' instead of 'void (* const)(const char *, ...)'
|
Pushed an update here to fix the underlying c type of the function parameter to be a const. This fixes it for me locally — @ninovanhooff can you give it a test? |
|
Unfortunately doesn't work yet for me on Mac. I did check your latest Regarding GCC 14; I can't remember when I installed that, and it seems in fact I did not. note that clang is used. It is supplied by the Xcode toolchain, and might be updated automatically when XCode is updated. Note: I also performed a Sanity check on Windows, where GCC 10.3.0 is installed. Indeed, it works fine there |
|
Good to know — I tested against gcc. I’ll test against clang next |
|
Hold the phone... I decided to add a github action that compiles the example project on mac os. |
|
Ok, so I organised my thoughts. The reason my action was succeeding is that it was based on samdze/dev and not Nycto:elideMemtrace. So that means we have a passing baseline run And a failing run that is based on the elide branch |
|
You may test fixes by branching off Nycto#1 Unfortunately, it seems the action compiles the nim sources rather than installing prebuilt, and doesn't cache the results. So the run might take some time |
|
Good news! This was an easy fix. I just needed to share the same type definition everywhere it’s used. |
This change does some code organization, but most importantly it completely removes the memory tracing code when it isn't enabled. This is likely causing inflated memory consumption for games using the bindings at the moment, as it allocates memory for the memtrace regardless of whether tracing is enabled.