libsass icon indicating copy to clipboard operation
libsass copied to clipboard

Optimize big stack allocation (move to heap)

Open mgreter opened this issue 4 years ago • 5 comments

mgreter avatar Oct 23 '19 19:10 mgreter

Stack allocation is free but heap isn't; how is this an optimization?

glebm avatar Oct 23 '19 20:10 glebm

Stack allocation is free but heap isn't; how is this an optimization?

Out of stack issues? https://docs.microsoft.com/en-us/visualstudio/code-quality/c6262?view=vs-2019

mgreter avatar Oct 23 '19 20:10 mgreter

@glebm care to create a PR with your suggested changes!?

mgreter avatar Oct 27 '19 01:10 mgreter

Stack allocation is free but heap isn't; how is this an optimization?

Out of stack issues? https://docs.microsoft.com/en-us/visualstudio/code-quality/c6262?view=vs-2019

There is nothing wrong per se in having 32KB on stack. We could add an option to have /analyze:stacksize40000 or something.

Did this cause real-life crashes of libsass due to running out of stack? Even Alpine Linux lets us having more stack ... (but this code does not apply to Linux).

saper avatar Oct 28 '19 13:10 saper

The memory payload is actually 64k, since the allocated array is of wchar. In the end this is just a precaution to satisfy MSVC static code analysis. But I think it does make sense to allocate those 64k on the heap!?

mgreter avatar Jan 17 '20 02:01 mgreter