Re: [Mesa-dev] Question: st/mesa and context-shareable shaders
On Wed, Sep 30, 2015 at 3:53 PM, Roland Scheidegger wrote: > Am 30.09.2015 um 11:41 schrieb Erik Faye-Lund: >> On Mon, Sep 28, 2015 at 4:39 PM, Roland Scheidegger >> wrote: >>> >>> In short, for simplicity, only things were sharable which were really >>> required to be shared (pretty much just actual resources - and yes that >>> doesn't work too well for GL neither as you can't share sampler/rt >>> views, let's face it GL's resource model there from legacy GL is a >>> disaster, not that d3d9 was any better). >> >> OpenGL allows sharing the objects that potentially take up a lot of >> sharable memory, like shaders, programs, texture images, renderbuffers >> and vertex buffer objects. But not those who does not, like >> framebuffer objects and sampler objects. This makes a ton of sense to >> me, and calling this model "a disaster" seems quite unfair. > The "disaster" was in reference to the separate textures/renderbuffers, > which imho really is a disaster - of course this has its root in actual > implementations (which sometimes even had separate memory for those). It > continues to make problems everywhere, there's still some ugly hacks for > instance in the mesa statetracker which we can't get rid of (because > some optimal format for a texture might then not be renderable). So yes, > "resources" need to be shareable (albeit GL calls them vbos, textures, > renderbuffers). And yes these can take up a lot of memory. > Typically, shaders/programs were much smaller than these, albeit they've > grown considerably - don't forget gallium is more than 7 years old too. > So maybe that they are sharable in GL was a reasonable forward looking > choice, but gallium was meant to make things easier for drivers, and > typically there was no way you could compile things to hw without > context information in any case. > >> >>> And IIRC multithreaded GL in general never really used to work all that >>> well and noone was really using that much. >> >> That's not quite true. I've been writing multithreaded OpenGL >> applications professionally for the last 7 years, and I've experienced >> very few problems with the proprietary drivers in this respect. OpenGL >> multi-threading works fine, and is in rather heavy use out there. You >> might not see those applications, but they exist. And they have been >> existing for a long time, without notable problems. >> > There were some minimal demos in mesa for multithreaded GL, and IIRC > they didn't make problems only in mesa itself. Though granted maybe 7 > years isn't back far enough... I thought though the usefulness was > typically quite limited but it wouldn't be surprising if that changed > too (so if you actually compiled your shader in a different thread, I > wouldn't have been surprised if most of the compile time was actually > spent in the thread where you first used that shader anyway). Of course > 10 years back most people didn't even have dual-core cpus... Nowadays, a lot of applications use several contexts and threads such as web browsers (firefox, chrome) and game engines (UE4). Historically, web browsers suffered most from the sampler-view <-> context dependency and we had some ugly bugs due to that. (making sampler views per-screen would simplify that code a lot) The number of multithreaded GL applications is only going to grow and drivers must already be thread-safe to be considered usable. Also, piglit has a multithreaded shader-compiling glx test that has been very useful for fixing race conditions in our driver. We definitely need more of those. Marek ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Question: st/mesa and context-shareable shaders
Am 30.09.2015 um 11:41 schrieb Erik Faye-Lund: > On Mon, Sep 28, 2015 at 4:39 PM, Roland Scheidegger > wrote: >> >> In short, for simplicity, only things were sharable which were really >> required to be shared (pretty much just actual resources - and yes that >> doesn't work too well for GL neither as you can't share sampler/rt >> views, let's face it GL's resource model there from legacy GL is a >> disaster, not that d3d9 was any better). > > OpenGL allows sharing the objects that potentially take up a lot of > sharable memory, like shaders, programs, texture images, renderbuffers > and vertex buffer objects. But not those who does not, like > framebuffer objects and sampler objects. This makes a ton of sense to > me, and calling this model "a disaster" seems quite unfair. The "disaster" was in reference to the separate textures/renderbuffers, which imho really is a disaster - of course this has its root in actual implementations (which sometimes even had separate memory for those). It continues to make problems everywhere, there's still some ugly hacks for instance in the mesa statetracker which we can't get rid of (because some optimal format for a texture might then not be renderable). So yes, "resources" need to be shareable (albeit GL calls them vbos, textures, renderbuffers). And yes these can take up a lot of memory. Typically, shaders/programs were much smaller than these, albeit they've grown considerably - don't forget gallium is more than 7 years old too. So maybe that they are sharable in GL was a reasonable forward looking choice, but gallium was meant to make things easier for drivers, and typically there was no way you could compile things to hw without context information in any case. > >> And IIRC multithreaded GL in general never really used to work all that >> well and noone was really using that much. > > That's not quite true. I've been writing multithreaded OpenGL > applications professionally for the last 7 years, and I've experienced > very few problems with the proprietary drivers in this respect. OpenGL > multi-threading works fine, and is in rather heavy use out there. You > might not see those applications, but they exist. And they have been > existing for a long time, without notable problems. > There were some minimal demos in mesa for multithreaded GL, and IIRC they didn't make problems only in mesa itself. Though granted maybe 7 years isn't back far enough... I thought though the usefulness was typically quite limited but it wouldn't be surprising if that changed too (so if you actually compiled your shader in a different thread, I wouldn't have been surprised if most of the compile time was actually spent in the thread where you first used that shader anyway). Of course 10 years back most people didn't even have dual-core cpus... Roland ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Question: st/mesa and context-shareable shaders
On Mon, Sep 28, 2015 at 4:39 PM, Roland Scheidegger wrote: > > In short, for simplicity, only things were sharable which were really > required to be shared (pretty much just actual resources - and yes that > doesn't work too well for GL neither as you can't share sampler/rt > views, let's face it GL's resource model there from legacy GL is a > disaster, not that d3d9 was any better). OpenGL allows sharing the objects that potentially take up a lot of sharable memory, like shaders, programs, texture images, renderbuffers and vertex buffer objects. But not those who does not, like framebuffer objects and sampler objects. This makes a ton of sense to me, and calling this model "a disaster" seems quite unfair. > And IIRC multithreaded GL in general never really used to work all that > well and noone was really using that much. That's not quite true. I've been writing multithreaded OpenGL applications professionally for the last 7 years, and I've experienced very few problems with the proprietary drivers in this respect. OpenGL multi-threading works fine, and is in rather heavy use out there. You might not see those applications, but they exist. And they have been existing for a long time, without notable problems. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Question: st/mesa and context-shareable shaders
On Mon, Sep 28, 2015 at 11:59 PM, Jose Fonseca wrote: > On 28/09/15 21:21, Marek Olšák wrote: >> >> On Mon, Sep 28, 2015 at 9:28 PM, Jose Fonseca wrote: >>> >>> On 28/09/15 14:51, Marek Olšák wrote: On Mon, Sep 28, 2015 at 2:55 PM, Jose Fonseca wrote: > > > On 27/09/15 19:14, Marek Olšák wrote: >> >> >> >> Hi, >> >> For some reason, st/mesa assumes that shaders can't be shared by >> multiple contexts and therefore has a context pointer in shader keys. >> I'd like to get rid of this dependency, because there is no reason why >> shaders would need to be tied to a context. In fact, shaders don't >> even need a device, they just need a compiler. > > > >> >> >> This is becoming a bigger issue with latest games that might prefer >> compiling shaders in another thread using a different GL context. >> >> As far as I know, shaders should be context-shareable on all hardware >> drivers. > > > > > The thing to keep in mind is that, although in theory gallium shaders > could > be compiled once and reused by many pipe contexts, in practice many > drivers > need to produce shader variants that mix in state from other context > specific state objects, at draw time. > > llvmpipe, svga pipe driver are examples of such drivers that I know. > Maybe > amd/nvidia gallium drivers are immune from that problem, but my > understanding was that very few hardware has internal state objects > that > line up perfectly with gallium state objects, hence that this problem > was > widespread. > > > And because until now shaders were per-context, this means that pipe > drivers > can attach the variants to the shader object without any worries about > multi-threading, as it's guaranteed that no other thread is operating > on > them. I'm well aware of that issue. >>> >>> >>> >>> If you're aware but didn't think worth mention, then you clearly are >>> underestimating the issue. >>> >>> Multi-threading is never easy. I think it's worth for you to open your >>> mind >>> and think things through more thoroughly. >> >> >> I thought about it very thoroughly and I'm quite sure about it. See below. >> >>> You can use a mutex to prevent 2 threads from adding 2 shader variants simultaneously into a pipe shader. This should prevent corrupting the linked list of variants. Looking up the required shader variant based on states doesn't need locks if you add an atomic counter containing how many shaders are in the list, preventing the lookup function from having to read the ending pointer of the list where new shaders can be added by other threads. Drivers also shouldn't attach state pointers to shaders, but instead they should add their copies there. All in all, this is quite easily fixable and therefore not a big issue. It certainly doesn't prevent shaders from becoming screen objects. >>> >>> >>> >>> > > So, if you want to make pipe shaders global, you need to ensure that > all > shaders objects manipulations are thread-safe. In particular, if you > still > want to attach variants to the shaders, you need to ensure access to > mutable > members are protected with mutex. Which means you might be forced to > lock > mutexes on hot-paths, severely impacting performance (single- and > multi- > thread apps alike). The mutex would only have to be locked when a new shader variant is added, which is a time-consuming operation by itself (compilation etc.), so there is no performance concern from adding a mutex. >>> >>> >>> Other cases don't need the lock. >>> >>> >>> I don't see how that can work. >>> >>> You need to lock, not only when adding variants, but _all_ accesses, >>> including traversing the variants to search for the right one. >>> >>> That is, _every_ time you need to search for a variant in the screen >>> shader, >>> you'll need to lock the shader. If you don't, a different thread might >>> write to the variant list/hashtable exactly at the same time you're >>> traversing it, invalidating the pointers you're using to iterate over the >>> variants. >>> >>> In other words, you'll be locking mutexes on pretty much every draw >>> call... >>> >>> Personally I think that's a poor design. I don't know how much is the >>> mutex >>> lock overhead is in practice nowadays, but doing it per draw seems bad, >>> because it could all be avoided with a better design. >> >> >> No, I don't need the lock while traversing the variants. I don't use a >> hash table or a linked list from util. Our shader state has a pointer >> to the first shader variant. The variant itself has a pointer to the >> next variant. As long as I keep an atomic counter containing the >> number of variants,
Re: [Mesa-dev] Question: st/mesa and context-shareable shaders
On 28/09/15 15:30, Rob Clark wrote: On Mon, Sep 28, 2015 at 8:55 AM, Jose Fonseca wrote: I see only 2 options out of this: 1) Removing the context pointer from the shader keys. If drivers need this, they should handle it by themselves. This will simplify st/mesa, because TCS, TES, GS won't need the variant-tracking machinery. 2) Adding PIPE_CAP_SHAREABLE_SHADERS and if the cap is 1, set the context pointer to NULL in all keys. how would this look from gallium API? Move pipe->create_{fs,vs,gs,etc}_state() to screen (and take a pipe_screen ptr instead of pipe_context ptr)? fwiw, I mostly just (other than at emit time) use the pipe_context ptr to get at the pipe_screen. So other than sorting you the threading/locking issues it would seem straightforward to move.. What do you think? Marek I think there are other options worth considering: a) I wonder if wouldn't be better the other way around: if drivers want to avoid compiling the same shader for many contexts, they should maintain a shader cache internally that's shared by all contexts. As that can be done much more efficient -- there are many ways to implement a global cache that avoids frequent locking And the way to avoid reimplementing the wheel on every driver would be to have an helper module that would take care of that -- ie, a generial auxiliary module to maintain the shader cache in a thread safe maner. That module could potentially even learn to (de)serializing the cache from disk. b) split pipe shader objects in two parts: 1) a global object, which would allow the pipe drivers to pre-compile the TGSI into an internal IR 2) per-context objects, so that pipe drivers could produce variants of that IR which takes in account the currently bound state even these don't have to be per-context.. it's really just ==> variant.. I'd want to share the variants across context too.. Right. You could use Marek's idea and attach the variants to the screen-shader object internally too. Nothing is lost. But if a driver prefers to attach variants to the context-shader object, due to the simpler threading, or because the variants can be derived cheaply, or because there are so many variants that a linear search is not practical, it still can. And for pipe drivers that can't pre-compile anything, 1) would be nothing more than a duplication of TGSI tokens. c) introduce Metal/DX12/Vulkan like "pipeline state objects", whereby the state tracker declares upfront how the whole pipeline state objects are bound together, so the driver can produce the shader variant when that pipeline state is created, and doesn't need to worry about shader variantes at draw time. This would be useful for other reasons.. maybe not so much to avoid late variant generation at draw time, or at least my shader variants depend on raster and texture state. And I guess other drivers have their own weird variant state dependencies. llvmpipe is similar in some way. Many drivers suffer from similar problems -- many CSO end up being dumb structures which drivers copy around, just to be translated at draw time, when all state is known. We could craft the gallium pipeline state objects so that they would contain all information that drivers may ever need. The burden of copying and assembling the bits of state around would fall into the state tracker. Once the pipeline state object cache is populated, draw should succeed fast. But for freedreno, once we start getting gs/tes support, I'll want to generate binning pass shader variants with link time optimization.. BR, -R Jose ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Question: st/mesa and context-shareable shaders
On 28/09/15 21:21, Marek Olšák wrote: On Mon, Sep 28, 2015 at 9:28 PM, Jose Fonseca wrote: On 28/09/15 14:51, Marek Olšák wrote: On Mon, Sep 28, 2015 at 2:55 PM, Jose Fonseca wrote: On 27/09/15 19:14, Marek Olšák wrote: Hi, For some reason, st/mesa assumes that shaders can't be shared by multiple contexts and therefore has a context pointer in shader keys. I'd like to get rid of this dependency, because there is no reason why shaders would need to be tied to a context. In fact, shaders don't even need a device, they just need a compiler. This is becoming a bigger issue with latest games that might prefer compiling shaders in another thread using a different GL context. As far as I know, shaders should be context-shareable on all hardware drivers. The thing to keep in mind is that, although in theory gallium shaders could be compiled once and reused by many pipe contexts, in practice many drivers need to produce shader variants that mix in state from other context specific state objects, at draw time. llvmpipe, svga pipe driver are examples of such drivers that I know. Maybe amd/nvidia gallium drivers are immune from that problem, but my understanding was that very few hardware has internal state objects that line up perfectly with gallium state objects, hence that this problem was widespread. And because until now shaders were per-context, this means that pipe drivers can attach the variants to the shader object without any worries about multi-threading, as it's guaranteed that no other thread is operating on them. I'm well aware of that issue. If you're aware but didn't think worth mention, then you clearly are underestimating the issue. Multi-threading is never easy. I think it's worth for you to open your mind and think things through more thoroughly. I thought about it very thoroughly and I'm quite sure about it. See below. You can use a mutex to prevent 2 threads from adding 2 shader variants simultaneously into a pipe shader. This should prevent corrupting the linked list of variants. Looking up the required shader variant based on states doesn't need locks if you add an atomic counter containing how many shaders are in the list, preventing the lookup function from having to read the ending pointer of the list where new shaders can be added by other threads. Drivers also shouldn't attach state pointers to shaders, but instead they should add their copies there. All in all, this is quite easily fixable and therefore not a big issue. It certainly doesn't prevent shaders from becoming screen objects. So, if you want to make pipe shaders global, you need to ensure that all shaders objects manipulations are thread-safe. In particular, if you still want to attach variants to the shaders, you need to ensure access to mutable members are protected with mutex. Which means you might be forced to lock mutexes on hot-paths, severely impacting performance (single- and multi- thread apps alike). The mutex would only have to be locked when a new shader variant is added, which is a time-consuming operation by itself (compilation etc.), so there is no performance concern from adding a mutex. Other cases don't need the lock. I don't see how that can work. You need to lock, not only when adding variants, but _all_ accesses, including traversing the variants to search for the right one. That is, _every_ time you need to search for a variant in the screen shader, you'll need to lock the shader. If you don't, a different thread might write to the variant list/hashtable exactly at the same time you're traversing it, invalidating the pointers you're using to iterate over the variants. In other words, you'll be locking mutexes on pretty much every draw call... Personally I think that's a poor design. I don't know how much is the mutex lock overhead is in practice nowadays, but doing it per draw seems bad, because it could all be avoided with a better design. No, I don't need the lock while traversing the variants. I don't use a hash table or a linked list from util. Our shader state has a pointer to the first shader variant. The variant itself has a pointer to the next variant. As long as I keep an atomic counter containing the number of variants, I will only have to traverse that number of variants. The variants itself are immutable. The list itself is immutable except for the "next" pointer in the last variant, which I will never access without the lock, because the atomic counter tells me how many I have. If the driver doesn't find the variant in the list, it will have to acquire the lock, search the list again (see "double-checked locking optimization"), if it still can't find it, then it will compile a new variant and add it to the list. Then unlock. This is fairly lockless except for the compilation and even during the compilation, it remains lockless for contexts that only use already-compiled variants. However, I do understand that using a more co
Re: [Mesa-dev] Question: st/mesa and context-shareable shaders
On Mon, Sep 28, 2015 at 9:28 PM, Jose Fonseca wrote: > On 28/09/15 14:51, Marek Olšák wrote: >> >> On Mon, Sep 28, 2015 at 2:55 PM, Jose Fonseca wrote: >>> >>> On 27/09/15 19:14, Marek Olšák wrote: Hi, For some reason, st/mesa assumes that shaders can't be shared by multiple contexts and therefore has a context pointer in shader keys. I'd like to get rid of this dependency, because there is no reason why shaders would need to be tied to a context. In fact, shaders don't even need a device, they just need a compiler. >>> >>> This is becoming a bigger issue with latest games that might prefer compiling shaders in another thread using a different GL context. As far as I know, shaders should be context-shareable on all hardware drivers. >>> >>> >>> >>> The thing to keep in mind is that, although in theory gallium shaders >>> could >>> be compiled once and reused by many pipe contexts, in practice many >>> drivers >>> need to produce shader variants that mix in state from other context >>> specific state objects, at draw time. >>> >>> llvmpipe, svga pipe driver are examples of such drivers that I know. >>> Maybe >>> amd/nvidia gallium drivers are immune from that problem, but my >>> understanding was that very few hardware has internal state objects that >>> line up perfectly with gallium state objects, hence that this problem was >>> widespread. >>> >>> >>> And because until now shaders were per-context, this means that pipe >>> drivers >>> can attach the variants to the shader object without any worries about >>> multi-threading, as it's guaranteed that no other thread is operating on >>> them. >> >> >> I'm well aware of that issue. > > > If you're aware but didn't think worth mention, then you clearly are > underestimating the issue. > > Multi-threading is never easy. I think it's worth for you to open your mind > and think things through more thoroughly. I thought about it very thoroughly and I'm quite sure about it. See below. > >> You can use a mutex to prevent 2 threads >> >> from adding 2 shader variants simultaneously into a pipe shader. This >> should prevent corrupting the linked list of variants. Looking up the >> required shader variant based on states doesn't need locks if you add >> an atomic counter containing how many shaders are in the list, >> preventing the lookup function from having to read the ending pointer >> of the list where new shaders can be added by other threads. >> >> Drivers also shouldn't attach state pointers to shaders, but instead >> they should add their copies there. All in all, this is quite easily >> fixable and therefore not a big issue. It certainly doesn't prevent >> shaders from becoming screen objects. > > > >>> >>> So, if you want to make pipe shaders global, you need to ensure that all >>> shaders objects manipulations are thread-safe. In particular, if you >>> still >>> want to attach variants to the shaders, you need to ensure access to >>> mutable >>> members are protected with mutex. Which means you might be forced to >>> lock >>> mutexes on hot-paths, severely impacting performance (single- and multi- >>> thread apps alike). >> >> >> The mutex would only have to be locked when a new shader variant is >> added, which is a time-consuming operation by itself (compilation >> etc.), so there is no performance concern from adding a mutex. > > >> Other cases don't need the lock. > > I don't see how that can work. > > You need to lock, not only when adding variants, but _all_ accesses, > including traversing the variants to search for the right one. > > That is, _every_ time you need to search for a variant in the screen shader, > you'll need to lock the shader. If you don't, a different thread might > write to the variant list/hashtable exactly at the same time you're > traversing it, invalidating the pointers you're using to iterate over the > variants. > > In other words, you'll be locking mutexes on pretty much every draw call... > > Personally I think that's a poor design. I don't know how much is the mutex > lock overhead is in practice nowadays, but doing it per draw seems bad, > because it could all be avoided with a better design. No, I don't need the lock while traversing the variants. I don't use a hash table or a linked list from util. Our shader state has a pointer to the first shader variant. The variant itself has a pointer to the next variant. As long as I keep an atomic counter containing the number of variants, I will only have to traverse that number of variants. The variants itself are immutable. The list itself is immutable except for the "next" pointer in the last variant, which I will never access without the lock, because the atomic counter tells me how many I have. If the driver doesn't find the variant in the list, it will have to acquire the lock, search the list again (see "double-checked locking optimization"), if it still can't find it, then it wil
Re: [Mesa-dev] Question: st/mesa and context-shareable shaders
On 28/09/15 14:51, Marek Olšák wrote: On Mon, Sep 28, 2015 at 2:55 PM, Jose Fonseca wrote: On 27/09/15 19:14, Marek Olšák wrote: Hi, For some reason, st/mesa assumes that shaders can't be shared by multiple contexts and therefore has a context pointer in shader keys. I'd like to get rid of this dependency, because there is no reason why shaders would need to be tied to a context. In fact, shaders don't even need a device, they just need a compiler. This is becoming a bigger issue with latest games that might prefer compiling shaders in another thread using a different GL context. As far as I know, shaders should be context-shareable on all hardware drivers. The thing to keep in mind is that, although in theory gallium shaders could be compiled once and reused by many pipe contexts, in practice many drivers need to produce shader variants that mix in state from other context specific state objects, at draw time. llvmpipe, svga pipe driver are examples of such drivers that I know. Maybe amd/nvidia gallium drivers are immune from that problem, but my understanding was that very few hardware has internal state objects that line up perfectly with gallium state objects, hence that this problem was widespread. And because until now shaders were per-context, this means that pipe drivers can attach the variants to the shader object without any worries about multi-threading, as it's guaranteed that no other thread is operating on them. I'm well aware of that issue. If you're aware but didn't think worth mention, then you clearly are underestimating the issue. Multi-threading is never easy. I think it's worth for you to open your mind and think things through more thoroughly. > You can use a mutex to prevent 2 threads from adding 2 shader variants simultaneously into a pipe shader. This should prevent corrupting the linked list of variants. Looking up the required shader variant based on states doesn't need locks if you add an atomic counter containing how many shaders are in the list, preventing the lookup function from having to read the ending pointer of the list where new shaders can be added by other threads. Drivers also shouldn't attach state pointers to shaders, but instead they should add their copies there. All in all, this is quite easily fixable and therefore not a big issue. It certainly doesn't prevent shaders from becoming screen objects. So, if you want to make pipe shaders global, you need to ensure that all shaders objects manipulations are thread-safe. In particular, if you still want to attach variants to the shaders, you need to ensure access to mutable members are protected with mutex. Which means you might be forced to lock mutexes on hot-paths, severely impacting performance (single- and multi- thread apps alike). The mutex would only have to be locked when a new shader variant is added, which is a time-consuming operation by itself (compilation etc.), so there is no performance concern from adding a mutex. > Other cases don't need the lock. I don't see how that can work. You need to lock, not only when adding variants, but _all_ accesses, including traversing the variants to search for the right one. That is, _every_ time you need to search for a variant in the screen shader, you'll need to lock the shader. If you don't, a different thread might write to the variant list/hashtable exactly at the same time you're traversing it, invalidating the pointers you're using to iterate over the variants. In other words, you'll be locking mutexes on pretty much every draw call... Personally I think that's a poor design. I don't know how much is the mutex lock overhead is in practice nowadays, but doing it per draw seems bad, because it could all be avoided with a better design. One could avoid the threading primitives by attaching the variants to the context themselves -- but then that's no different from what we do today -- ie, you'd be compiling as often as today. Like I said above, it's easy to add shader variants to screen-based shaders with a mutex that has no impact on performance. I disagree with your option 1). But if you're still determined, by all means, do option 2). I think you'll regret it though: down the road you'll be either struggling with multi-thread correctness or performance. I don't feel too strongly either way: maybe the benefits of having shaders shared by many pipe context are indeed worth the trouble. But let it be no doubt: that implies making all operations on shader objects thread safe. And that is not a trivial matter. It's certainly not as easy as just coping with a null pipe_context argument. I see only 2 options out of this: 1) Removing the context pointer from the shader keys. If drivers need this, they should handle it by themselves. This will simplify st/mesa, because TCS, TES, GS won't need the variant-tracking machinery. 2) Adding PIPE_CAP_SHAREABLE_SHADERS and if the cap is 1, set th
Re: [Mesa-dev] Question: st/mesa and context-shareable shaders
Am 28.09.2015 um 15:51 schrieb Marek Olšák: > On Mon, Sep 28, 2015 at 2:55 PM, Jose Fonseca wrote: >> On 27/09/15 19:14, Marek Olšák wrote: >>> >>> Hi, >>> >>> For some reason, st/mesa assumes that shaders can't be shared by >>> multiple contexts and therefore has a context pointer in shader keys. >>> I'd like to get rid of this dependency, because there is no reason why >>> shaders would need to be tied to a context. In fact, shaders don't >>> even need a device, they just need a compiler. >> >>> >>> >>> This is becoming a bigger issue with latest games that might prefer >>> compiling shaders in another thread using a different GL context. >>> >>> As far as I know, shaders should be context-shareable on all hardware >>> drivers. >> >> >> The thing to keep in mind is that, although in theory gallium shaders could >> be compiled once and reused by many pipe contexts, in practice many drivers >> need to produce shader variants that mix in state from other context >> specific state objects, at draw time. >> >> llvmpipe, svga pipe driver are examples of such drivers that I know. Maybe >> amd/nvidia gallium drivers are immune from that problem, but my >> understanding was that very few hardware has internal state objects that >> line up perfectly with gallium state objects, hence that this problem was >> widespread. >> >> >> And because until now shaders were per-context, this means that pipe drivers >> can attach the variants to the shader object without any worries about >> multi-threading, as it's guaranteed that no other thread is operating on >> them. > > I'm well aware of that issue. You can use a mutex to prevent 2 threads > from adding 2 shader variants simultaneously into a pipe shader. This > should prevent corrupting the linked list of variants. Looking up the > required shader variant based on states doesn't need locks if you add > an atomic counter containing how many shaders are in the list, > preventing the lookup function from having to read the ending pointer > of the list where new shaders can be added by other threads. > > Drivers also shouldn't attach state pointers to shaders, but instead > they should add their copies there. All in all, this is quite easily > fixable and therefore not a big issue. It certainly doesn't prevent > shaders from becoming screen objects. > > >> >> >> So, if you want to make pipe shaders global, you need to ensure that all >> shaders objects manipulations are thread-safe. In particular, if you still >> want to attach variants to the shaders, you need to ensure access to mutable >> members are protected with mutex. Which means you might be forced to lock >> mutexes on hot-paths, severely impacting performance (single- and multi- >> thread apps alike). > > The mutex would only have to be locked when a new shader variant is > added, which is a time-consuming operation by itself (compilation > etc.), so there is no performance concern from adding a mutex. Other > cases don't need the lock. > >> >> >> One could avoid the threading primitives by attaching the variants to the >> context themselves -- but then that's no different from what we do today -- >> ie, you'd be compiling as often as today. > > Like I said above, it's easy to add shader variants to screen-based > shaders with a mutex that has no impact on performance. > >> >> >> >> I don't feel too strongly either way: maybe the benefits of having shaders >> shared by many pipe context are indeed worth the trouble. But let it be no >> doubt: that implies making all operations on shader objects thread safe. >> And that is not a trivial matter. It's certainly not as easy as just coping >> with a null pipe_context argument. >> >> >>> I see only 2 options out of this: >>> >>> 1) Removing the context pointer from the shader keys. If drivers need >>> this, they should handle it by themselves. This will simplify st/mesa, >>> because TCS, TES, GS won't need the variant-tracking machinery. >>> >>> 2) Adding PIPE_CAP_SHAREABLE_SHADERS and if the cap is 1, set the >>> context pointer to NULL in all keys. >>> >>> What do you think? >>> >>> Marek >> >> >> I think there are other options worth considering: >> >> >> a) I wonder if wouldn't be better the other way around: if drivers want to >> avoid compiling the same shader for many contexts, they should maintain a >> shader cache internally that's shared by all contexts. >> >>As that can be done much more efficient -- there are many ways to >> implement a global cache that avoids frequent locking >> >>And the way to avoid reimplementing the wheel on every driver would be to >> have an helper module that would take care of that -- ie, a generial >> auxiliary module to maintain the shader cache in a thread safe maner. That >> module could potentially even learn to (de)serializing the cache from disk. > > This is too complicated for now and not required for having > screen-based shaders. > >> >> >> b) split pipe shader objects in two parts: >> >>1) a g
Re: [Mesa-dev] Question: st/mesa and context-shareable shaders
On Mon, Sep 28, 2015 at 8:55 AM, Jose Fonseca wrote: > > >> I see only 2 options out of this: >> >> 1) Removing the context pointer from the shader keys. If drivers need >> this, they should handle it by themselves. This will simplify st/mesa, >> because TCS, TES, GS won't need the variant-tracking machinery. >> >> 2) Adding PIPE_CAP_SHAREABLE_SHADERS and if the cap is 1, set the >> context pointer to NULL in all keys. how would this look from gallium API? Move pipe->create_{fs,vs,gs,etc}_state() to screen (and take a pipe_screen ptr instead of pipe_context ptr)? fwiw, I mostly just (other than at emit time) use the pipe_context ptr to get at the pipe_screen. So other than sorting you the threading/locking issues it would seem straightforward to move.. >> What do you think? >> >> Marek > > > I think there are other options worth considering: > > > a) I wonder if wouldn't be better the other way around: if drivers want to > avoid compiling the same shader for many contexts, they should maintain a > shader cache internally that's shared by all contexts. > >As that can be done much more efficient -- there are many ways to > implement a global cache that avoids frequent locking > >And the way to avoid reimplementing the wheel on every driver would be to > have an helper module that would take care of that -- ie, a generial > auxiliary module to maintain the shader cache in a thread safe maner. That > module could potentially even learn to (de)serializing the cache from disk. > > > b) split pipe shader objects in two parts: > >1) a global object, which would allow the pipe drivers to pre-compile the > TGSI into an internal IR > >2) per-context objects, so that pipe drivers could produce variants of > that IR which takes in account the currently bound state even these don't have to be per-context.. it's really just ==> variant.. I'd want to share the variants across context too.. > And for pipe drivers that can't pre-compile anything, 1) would be nothing > more than a duplication of TGSI tokens. > > > > c) introduce Metal/DX12/Vulkan like "pipeline state objects", whereby the > state tracker declares upfront how the whole pipeline state objects are > bound together, so the driver can produce the shader variant when that > pipeline state is created, and doesn't need to worry about shader variantes > at draw time. This would be useful for other reasons.. maybe not so much to avoid late variant generation at draw time, or at least my shader variants depend on raster and texture state. And I guess other drivers have their own weird variant state dependencies. But for freedreno, once we start getting gs/tes support, I'll want to generate binning pass shader variants with link time optimization.. BR, -R ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Question: st/mesa and context-shareable shaders
On Mon, Sep 28, 2015 at 2:55 PM, Jose Fonseca wrote: > On 27/09/15 19:14, Marek Olšák wrote: >> >> Hi, >> >> For some reason, st/mesa assumes that shaders can't be shared by >> multiple contexts and therefore has a context pointer in shader keys. >> I'd like to get rid of this dependency, because there is no reason why >> shaders would need to be tied to a context. In fact, shaders don't >> even need a device, they just need a compiler. > >> >> >> This is becoming a bigger issue with latest games that might prefer >> compiling shaders in another thread using a different GL context. >> >> As far as I know, shaders should be context-shareable on all hardware >> drivers. > > > The thing to keep in mind is that, although in theory gallium shaders could > be compiled once and reused by many pipe contexts, in practice many drivers > need to produce shader variants that mix in state from other context > specific state objects, at draw time. > > llvmpipe, svga pipe driver are examples of such drivers that I know. Maybe > amd/nvidia gallium drivers are immune from that problem, but my > understanding was that very few hardware has internal state objects that > line up perfectly with gallium state objects, hence that this problem was > widespread. > > > And because until now shaders were per-context, this means that pipe drivers > can attach the variants to the shader object without any worries about > multi-threading, as it's guaranteed that no other thread is operating on > them. I'm well aware of that issue. You can use a mutex to prevent 2 threads from adding 2 shader variants simultaneously into a pipe shader. This should prevent corrupting the linked list of variants. Looking up the required shader variant based on states doesn't need locks if you add an atomic counter containing how many shaders are in the list, preventing the lookup function from having to read the ending pointer of the list where new shaders can be added by other threads. Drivers also shouldn't attach state pointers to shaders, but instead they should add their copies there. All in all, this is quite easily fixable and therefore not a big issue. It certainly doesn't prevent shaders from becoming screen objects. > > > So, if you want to make pipe shaders global, you need to ensure that all > shaders objects manipulations are thread-safe. In particular, if you still > want to attach variants to the shaders, you need to ensure access to mutable > members are protected with mutex. Which means you might be forced to lock > mutexes on hot-paths, severely impacting performance (single- and multi- > thread apps alike). The mutex would only have to be locked when a new shader variant is added, which is a time-consuming operation by itself (compilation etc.), so there is no performance concern from adding a mutex. Other cases don't need the lock. > > > One could avoid the threading primitives by attaching the variants to the > context themselves -- but then that's no different from what we do today -- > ie, you'd be compiling as often as today. Like I said above, it's easy to add shader variants to screen-based shaders with a mutex that has no impact on performance. > > > > I don't feel too strongly either way: maybe the benefits of having shaders > shared by many pipe context are indeed worth the trouble. But let it be no > doubt: that implies making all operations on shader objects thread safe. > And that is not a trivial matter. It's certainly not as easy as just coping > with a null pipe_context argument. > > >> I see only 2 options out of this: >> >> 1) Removing the context pointer from the shader keys. If drivers need >> this, they should handle it by themselves. This will simplify st/mesa, >> because TCS, TES, GS won't need the variant-tracking machinery. >> >> 2) Adding PIPE_CAP_SHAREABLE_SHADERS and if the cap is 1, set the >> context pointer to NULL in all keys. >> >> What do you think? >> >> Marek > > > I think there are other options worth considering: > > > a) I wonder if wouldn't be better the other way around: if drivers want to > avoid compiling the same shader for many contexts, they should maintain a > shader cache internally that's shared by all contexts. > >As that can be done much more efficient -- there are many ways to > implement a global cache that avoids frequent locking > >And the way to avoid reimplementing the wheel on every driver would be to > have an helper module that would take care of that -- ie, a generial > auxiliary module to maintain the shader cache in a thread safe maner. That > module could potentially even learn to (de)serializing the cache from disk. This is too complicated for now and not required for having screen-based shaders. > > > b) split pipe shader objects in two parts: > >1) a global object, which would allow the pipe drivers to pre-compile the > TGSI into an internal IR > >2) per-context objects, so that pipe drivers could produce variants of > that IR which takes
Re: [Mesa-dev] Question: st/mesa and context-shareable shaders
On 27/09/15 19:14, Marek Olšák wrote: Hi, For some reason, st/mesa assumes that shaders can't be shared by multiple contexts and therefore has a context pointer in shader keys. I'd like to get rid of this dependency, because there is no reason why shaders would need to be tied to a context. In fact, shaders don't even need a device, they just need a compiler. > This is becoming a bigger issue with latest games that might prefer compiling shaders in another thread using a different GL context. As far as I know, shaders should be context-shareable on all hardware drivers. The thing to keep in mind is that, although in theory gallium shaders could be compiled once and reused by many pipe contexts, in practice many drivers need to produce shader variants that mix in state from other context specific state objects, at draw time. llvmpipe, svga pipe driver are examples of such drivers that I know. Maybe amd/nvidia gallium drivers are immune from that problem, but my understanding was that very few hardware has internal state objects that line up perfectly with gallium state objects, hence that this problem was widespread. And because until now shaders were per-context, this means that pipe drivers can attach the variants to the shader object without any worries about multi-threading, as it's guaranteed that no other thread is operating on them. So, if you want to make pipe shaders global, you need to ensure that all shaders objects manipulations are thread-safe. In particular, if you still want to attach variants to the shaders, you need to ensure access to mutable members are protected with mutex. Which means you might be forced to lock mutexes on hot-paths, severely impacting performance (single- and multi- thread apps alike). One could avoid the threading primitives by attaching the variants to the context themselves -- but then that's no different from what we do today -- ie, you'd be compiling as often as today. I don't feel too strongly either way: maybe the benefits of having shaders shared by many pipe context are indeed worth the trouble. But let it be no doubt: that implies making all operations on shader objects thread safe. And that is not a trivial matter. It's certainly not as easy as just coping with a null pipe_context argument. > I see only 2 options out of this: > > 1) Removing the context pointer from the shader keys. If drivers need > this, they should handle it by themselves. This will simplify st/mesa, > because TCS, TES, GS won't need the variant-tracking machinery. > > 2) Adding PIPE_CAP_SHAREABLE_SHADERS and if the cap is 1, set the > context pointer to NULL in all keys. > > What do you think? > > Marek I think there are other options worth considering: a) I wonder if wouldn't be better the other way around: if drivers want to avoid compiling the same shader for many contexts, they should maintain a shader cache internally that's shared by all contexts. As that can be done much more efficient -- there are many ways to implement a global cache that avoids frequent locking And the way to avoid reimplementing the wheel on every driver would be to have an helper module that would take care of that -- ie, a generial auxiliary module to maintain the shader cache in a thread safe maner. That module could potentially even learn to (de)serializing the cache from disk. b) split pipe shader objects in two parts: 1) a global object, which would allow the pipe drivers to pre-compile the TGSI into an internal IR 2) per-context objects, so that pipe drivers could produce variants of that IR which takes in account the currently bound state And for pipe drivers that can't pre-compile anything, 1) would be nothing more than a duplication of TGSI tokens. c) introduce Metal/DX12/Vulkan like "pipeline state objects", whereby the state tracker declares upfront how the whole pipeline state objects are bound together, so the driver can produce the shader variant when that pipeline state is created, and doesn't need to worry about shader variantes at draw time. This is in fact like a) but moves the burden of creating the variant cache into the state tracker (as the pipeline state cache) where it can be shared by all pipe drivers. Jose > For some reason, st/mesa assumes that shaders can't be shared by > multiple contexts and therefore has a context pointer in shader keys. PS: As I write this, I can't help having a feeling of deja-vu. I couldn't find any discussion from my mail archives, but I think this subject has been brought before. I wonder if we should start maintaining a "design.rst" in src/gallium/docs/ to document why some things were done the way they were. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Question: st/mesa and context-shareable shaders
On Sun, Sep 27, 2015 at 10:41 PM, Ilia Mirkin wrote: > On Sun, Sep 27, 2015 at 2:25 PM, Marek Olšák wrote: >> On Sun, Sep 27, 2015 at 8:21 PM, Ilia Mirkin wrote: >>> On Sun, Sep 27, 2015 at 2:14 PM, Marek Olšák wrote: Hi, For some reason, st/mesa assumes that shaders can't be shared by multiple contexts and therefore has a context pointer in shader keys. I'd like to get rid of this dependency, because there is no reason why shaders would need to be tied to a context. In fact, shaders don't even need a device, they just need a compiler. This is becoming a bigger issue with latest games that might prefer compiling shaders in another thread using a different GL context. As far as I know, shaders should be context-shareable on all hardware drivers. I see only 2 options out of this: 1) Removing the context pointer from the shader keys. If drivers need this, they should handle it by themselves. This will simplify st/mesa, because TCS, TES, GS won't need the variant-tracking machinery. 2) Adding PIPE_CAP_SHAREABLE_SHADERS and if the cap is 1, set the context pointer to NULL in all keys. What do you think? >>> >>> FWIW I believe this should work for at least nv50 and nvc0, as they >>> have a screen-shared code segment that all shaders get uploaded to. >>> However I believe that it would be crucial to have a piglit test that >>> exercises this functionality, as debugging this stuff esp in an actual >>> game is extremely difficult. >> >> Who needs to debug it. If there's a regression, it's bisectable. After >> that, you know your shaders aren't shareable and you need to take a >> closer look at your code. > > OK, well, in that case I'm just going to set such a cap to 0 until a > test appears -- I'm not sufficiently confident in how it all works to > say that it's definitely fine on nouveau. There are enough bugs that > failures from such an issue may not be easily bisected, or it may not > even be clear that old versions worked. Yeah, I'll probably add some test for simple sharing, i.e. one context compiles it and another one uses it. Marek ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Question: st/mesa and context-shareable shaders
On Sun, Sep 27, 2015 at 2:25 PM, Marek Olšák wrote: > On Sun, Sep 27, 2015 at 8:21 PM, Ilia Mirkin wrote: >> On Sun, Sep 27, 2015 at 2:14 PM, Marek Olšák wrote: >>> Hi, >>> >>> For some reason, st/mesa assumes that shaders can't be shared by >>> multiple contexts and therefore has a context pointer in shader keys. >>> I'd like to get rid of this dependency, because there is no reason why >>> shaders would need to be tied to a context. In fact, shaders don't >>> even need a device, they just need a compiler. >>> >>> This is becoming a bigger issue with latest games that might prefer >>> compiling shaders in another thread using a different GL context. >>> >>> As far as I know, shaders should be context-shareable on all hardware >>> drivers. I see only 2 options out of this: >>> >>> 1) Removing the context pointer from the shader keys. If drivers need >>> this, they should handle it by themselves. This will simplify st/mesa, >>> because TCS, TES, GS won't need the variant-tracking machinery. >>> >>> 2) Adding PIPE_CAP_SHAREABLE_SHADERS and if the cap is 1, set the >>> context pointer to NULL in all keys. >>> >>> What do you think? >> >> FWIW I believe this should work for at least nv50 and nvc0, as they >> have a screen-shared code segment that all shaders get uploaded to. >> However I believe that it would be crucial to have a piglit test that >> exercises this functionality, as debugging this stuff esp in an actual >> game is extremely difficult. > > Who needs to debug it. If there's a regression, it's bisectable. After > that, you know your shaders aren't shareable and you need to take a > closer look at your code. OK, well, in that case I'm just going to set such a cap to 0 until a test appears -- I'm not sufficiently confident in how it all works to say that it's definitely fine on nouveau. There are enough bugs that failures from such an issue may not be easily bisected, or it may not even be clear that old versions worked. -ilia ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Question: st/mesa and context-shareable shaders
On Sun, Sep 27, 2015 at 8:21 PM, Ilia Mirkin wrote: > On Sun, Sep 27, 2015 at 2:14 PM, Marek Olšák wrote: >> Hi, >> >> For some reason, st/mesa assumes that shaders can't be shared by >> multiple contexts and therefore has a context pointer in shader keys. >> I'd like to get rid of this dependency, because there is no reason why >> shaders would need to be tied to a context. In fact, shaders don't >> even need a device, they just need a compiler. >> >> This is becoming a bigger issue with latest games that might prefer >> compiling shaders in another thread using a different GL context. >> >> As far as I know, shaders should be context-shareable on all hardware >> drivers. I see only 2 options out of this: >> >> 1) Removing the context pointer from the shader keys. If drivers need >> this, they should handle it by themselves. This will simplify st/mesa, >> because TCS, TES, GS won't need the variant-tracking machinery. >> >> 2) Adding PIPE_CAP_SHAREABLE_SHADERS and if the cap is 1, set the >> context pointer to NULL in all keys. >> >> What do you think? > > FWIW I believe this should work for at least nv50 and nvc0, as they > have a screen-shared code segment that all shaders get uploaded to. > However I believe that it would be crucial to have a piglit test that > exercises this functionality, as debugging this stuff esp in an actual > game is extremely difficult. Who needs to debug it. If there's a regression, it's bisectable. After that, you know your shaders aren't shareable and you need to take a closer look at your code. Marek ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Question: st/mesa and context-shareable shaders
On Sun, Sep 27, 2015 at 2:14 PM, Marek Olšák wrote: > Hi, > > For some reason, st/mesa assumes that shaders can't be shared by > multiple contexts and therefore has a context pointer in shader keys. > I'd like to get rid of this dependency, because there is no reason why > shaders would need to be tied to a context. In fact, shaders don't > even need a device, they just need a compiler. > > This is becoming a bigger issue with latest games that might prefer > compiling shaders in another thread using a different GL context. > > As far as I know, shaders should be context-shareable on all hardware > drivers. I see only 2 options out of this: > > 1) Removing the context pointer from the shader keys. If drivers need > this, they should handle it by themselves. This will simplify st/mesa, > because TCS, TES, GS won't need the variant-tracking machinery. > > 2) Adding PIPE_CAP_SHAREABLE_SHADERS and if the cap is 1, set the > context pointer to NULL in all keys. > > What do you think? FWIW I believe this should work for at least nv50 and nvc0, as they have a screen-shared code segment that all shaders get uploaded to. However I believe that it would be crucial to have a piglit test that exercises this functionality, as debugging this stuff esp in an actual game is extremely difficult. > > Marek > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev