Allocate memory for benchmarkPolyfill on the heap #198
Conversation
|
Great news, on replicating the error and change. Separate but somewhat related question: |
Allocating on a stack is always going to be a bit of a wild card -- you're eating into same memory you use to make function calls, so the amount you can safely allocate varies based on how deep in the stack you are. (Apparently 64-bit Windows consumes more of the stack before your program runs than 32-bit Windows and Unix-like OSes.) It's okay for small memory allocations because it can be faster than heap and is similar in a sense to just more arguments to your function, but for dynamic memory allocations it's not a great idea. We already removed usage of @isaacbrodsky I wasn't going to bring this up, but does this change remove the last calls to |
That's more of a question for @TengHu but it's definitely doable. I recently added Windows support to h3-node so if my free time permits I could tackle that. |
It is not: it's still used in some tests. I think we could entirely remove it (and supporting build code), since that may help eliminate this type of issue in the future. |
👍 |
Allocate memory for benchmarkPolyfill on the heap
Fixes #197
It looks like it's failing as it allocates something on the stack (usually for procedure entry, in this case for stack allocation), pointing to a problem with
STACK_ARRAY_CALLOC. I changed the benchmark to allocate on the heap instead. This does mean the benchmark now also covers allocating the memory on the heap, that could be removed from the benchmark by moving the call tomaxPolyfillSizeoutside the benchmark.