You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
After reading the patch, I realize it's not very helpful for our purposes.
The context buffer is a "scratch buffer" that is only allocated once and reused for all scratch purposes for the context lifetime.
The problem we were facing is that it is not thread safe.
My idea was to make it thread local, so each thread gets its own scratch buffer and everything is happy.
But I missed that you can't create a tss, bind it to the context lifetime, and have the context destructor free the buffers for all threads (i.e. call xkb_context_destroy_buffer only once in xkb_context_unref).
This means we have to match an xkb_context_destroy_buffer to each xkb_context_create_buffer. Which means the buffer isn't reused...
Obviously I haven't really used the tss_ API before making my suggestion, sorry about that.
So my new suggestion is, let's scratch the scratch buffer? That is, replace the xkb_context_create_buffer/xkb_context_destroy_buffer with malloc/free. This also doesn't reuse the buffer, but it's straightforward and no threading stuff :)
Just to be clear, I do not mean to remove the idea of the buffer entirely, i.e. use malloc/free for each xkb_context_get_buffer. That would indeed introduce too many allocations. I mean allocate a buffer per text_v1_keymap_get_as_string/text_v1_keymap_new_from_names call. That seems fine to me. The hassle with this is that we'd need to pass this buffer to all of the *Text functions, siText in particular inside xkbcomp/...
Other alternatives:
Make the buffer thread-global not bound to context lifetime. I don't like globals...
Allocate it on the stack. The buffer size is 2048 bytes (it needs to be able to fit whatever is thrown into it), maybe it's fine for today's stack sizes?
WDYT?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
compile-keymapIndicates a need for improvements or additions to keymap compilation
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Try to do minimal changes.
Partially fixes #300.