dix: Usage preferred allocation memory on stack

[GermanAizek]

@metux,
There are many changes in dix core that aim to use stack more logically for a small amount data than heap.
I also noticed a bug that grab->xi2mask is freed by usual free(), not xi2mask_free()

References:

• https://publicwork.wordpress.com/2019/06/27/stack-allocation-vs-heap-allocation-performance-benchmark/
• https://stackoverflow.com/a/56798629

[GermanAizek]

@metux,
all tests have been passed successfully, assign code reviewers.

[GermanAizek]

Added new commit.
Again all tests have been passed successfully.

[metux]

Already been there myself, but had to pull back, because unfortunately alloca() isn't safe at all:

It can't detect a stack overflow (at least not portably) and could hand out invalid memory regions (outside of call stack). See: https://man7.org/linux/man-pages/man3/alloca.3.html

Something we could do is using a per-request arena allocator, which can only allocate or clear/destroy the whole arena. Then we could have one arena per client that's cleared (not destroyed) right after the request dispatch had returned - now all request handlers could just allocate their temporary space from there w/o having to care about free'ing anymore.

Here's a little PoC for this:
https://github.com/metux/libephemeron

OTOH, not sure whether it's really worth that ... we should discuss this more closely
@algrid @b-aaz @josephcrowell @cepelinas9000 @stefan11111 @X11Libre/dev

[stefan11111]

As I see it, alloca should never be used, because there is no way to detect memory allocation issues.
From the man page:

RETURN VALUE
       The alloca() function returns a pointer to the beginning of the
       allocated space.  If the allocation causes stack overflow, program
       behavior is undefined.

The way I read this, an alloca implementation is free to simply align and subtract our requested size from rsp, and give us back that address.

Also note that the stack is usually only a few MB large, and we need the stack for more than just memory allocations, so we can easily overflow it.

While this us pretty obvious, we also have to be careful to not replace some *alloc with alloca, and then return that from a function.

I haven't checked the surrounding code, but for small allocations like this, we could instead do: unsigned char mem[2]; unsigned char *dash = mem;, and clear it if we need to.

[algrid]

Indeed, if it’s a small fixed size, I would just declare a variable. Simple and no need to care about freeing it.
In general what’s the reasoning behind switching to alloca? Is that piece of code extremely important for performance?

[metux]

Back when I attempted the same, my idea was
a) get rid of explicit free()'s (also simplify error pathes)
b) faster allocation -> alloca() just moving the stack pointer.

But I had to drop the idea, because alloca() is very fragile.

[metux]

if alloca() was safe, then it would be a good thing to do.

[metux]

I doubt it's worth this:

• the free()'s only happing rarely
• atom creation is pretty rare
• atoms are never destroyed
• we're thinking about using a different implementation (eg generic hashtable) anyways