On Thu, 2015-08-06 at 17:10 -0700, Matt Turner wrote: > Patches 1-11 improve performance of SynMark OglBatch7 by 6.29586% +/- > 0.277734% > (n=337) and OglMultithread by 1.12564% +/- 0.424038% (n=209). I haven't > benchmarked individual patches because I'd like to not waste all that time > if I > get review feedback that requires me to change things. :) > > Patches 12-13 were supposed to improve performance, but seem to make an > existing thread-safety problem worse, so I'm not proposing them for > inclusion. > > > [01/13] c11/threads: Assert that mtx is non-NULL and check > [02/13] mesa: Remove debugging code from _mesa_reference_*. >
Small comment on 1 but doesn't really matter. These two are Reviewed-by: Timothy Arceri <t_arc...@yahoo.com.au> > [03/13] mesa: Add locking to sampler objects. > [04/13] mesa: Add locking to programs. > > These two add missing locks to sampler and program objects, which > I believe are supposed to be thread-safe. These two look correct but shouldn't you just squash these with 6 and 7? If you really want to show the code evolution in git history then: Reviewed-by: Timothy Arceri <t_arc...@yahoo.com.au> > > [05/13] mesa: Replace buffer object locks with atomic inc/dec. > [06/13] mesa: Replace sampler object locks with atomic inc/dec. > [07/13] mesa: Replace program locks with atomic inc/dec. > [08/13] mesa: Replace renderbuffer object locks with atomic inc/dec. > [09/13] mesa: Replace texture buffer object locks with atomic inc/dec. > > These five replace locks around RefCount++/-- with atomic increment > and decrement. Nice I didn't know about this. Reviewed-by: Timothy Arceri <t_arc...@yahoo.com.au> > > [10/13] hash: Add _mesa_HashRemoveLocked() function. > [11/13] mesa: Replace uses of Shared->Mutex with hash-table mutexes > > These two replace uses of ctx->Shared->Mutex with the mutexes in the > hash tables. Nice. I was looking in this area early in the year when I noticed some common Phoronix benchmark games spent a lot of time locking in these areas so maybe some real work gains here. Some minor comments on these. Reviewed-by: Timothy Arceri <t_arc...@yahoo.com.au> > > [12/13] mesa: Remove unnecessary locking from container > [13/13] mesa: Remove deleteFlag pattern. > > *I am not proposing these for inclusion* > > These two remove some "unnecessary" locking from so called "container > objects" that are not shared between threads by the GL. While I expected > them to improve performance, they actually cause double-free errors in > SynMark OglMultithread. Valgrind's helgrind tool shows that there are > many > thread-safety issues in the texture code, and removing these locks seems > > to > exacerbate the problem. > > Specifically, multiple threads are reading and writing to > gl_texture_objects > without any synchronization from places like > intel_finalize_mipmap_tree(), > gen7_update_texture_surface(), brw_populate_sampler_prog_key_data(), > update_sampler_state(), and _mesa_BindTexture(). > > Suggestions for solving this (apparently quite longstanding) problem are > welcome. > _______________________________________________ > 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
