Linux: Moves DRM LRU FD cache to be per-thread #5211
Conversation
| class LRUCacheFDCache { | ||
| public: | ||
| using HandlerType = uint32_t (*)(FEXCore::Core::CpuStateFrame* Frame, int fd, uint32_t cmd, uint32_t args); | ||
| virtual ~LRUCacheFDCache() = default; |
There was a problem hiding this comment.
This doesn't actually need to be virtual, does it? There's only one implementation, and it only adds LRUSize/LRUCache/FDToHandler here, so we can just move it over.
There was a problem hiding this comment.
Moved as much to the header as possible without leaking DRM implementation details.
There was a problem hiding this comment.
This seems to have missed the intent of my comment. Instead of having two classes (one virtual, one "impl"), we can just have one. The function implementations themselves should stay in the cpp file (as opposed to their declarations).
Maybe I'm missing something that prevents all virtual from being dropped here, if so would appreciate taking a moment to explain.
There was a problem hiding this comment.
Indeed, there are some private DRM details in the AddandRunHandler that can't be exposed in the header.
There was a problem hiding this comment.
In the implementation of AddandRunHandler? So just declare the function in the header and implement it in the cpp file then?
Again I'm not suggesting moving any of the actual implementations into the header.
This was a nasty race condition where each thread could be accessing the DRM cache at any given moment. Move it over to a per thread object that is only allocated once it gets used. Also removes an `atexit` registration that contributes to crashing on exit.
3 participants
This was a nasty race condition where each thread could be accessing the DRM cache at any given moment. Move it over to a per thread object that is only allocated once it gets used.
Also removes an
atexitregistration that contributes to crashing on exit.