Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Redundant glXGetProcAddressARB-calls severely reducing performance #171

Closed
thrimbor opened this issue May 2, 2018 · 9 comments · Fixed by #173
Closed

Redundant glXGetProcAddressARB-calls severely reducing performance #171

thrimbor opened this issue May 2, 2018 · 9 comments · Fixed by #173

Comments

@thrimbor
Copy link

thrimbor commented May 2, 2018

This may be something stupid on my side, but maybe someone can shed some light on this.

I recently tried to use libepoxy in an attempt to port an SDL+OpenGL based game to Windows (the relevant commit is this one).
However, using libepoxy on Linux resulted in a drastic performance loss: The fps dropped from over 3000 to ~500, even dropping below 30 in some areas - which is extremely strange for such a simple renderer.

Further analysis showed that libepoxy does a significant amount of redundant GL-calls (glXGetCurrentContext, glGetString and glXGetProcAddressARB) before every call to an OpenGL function (here is an analysis of a single frame).

I ran it on Arch Linux with a GTX 970 (390.48 driver), and I tried libepoxy 1.5.0 and 1.5.1 (and 1.4.0 a while back, I think). Arch Linux doesn't seem to do anything special when building the package.

I'm not sure what the problem is here, I expected libepoxy to only query the addresses once. Are there known problems combining libepoxy with SDL (version 1), or am I hitting a bug here?

@ebassi
Copy link
Collaborator

ebassi commented May 5, 2018

Epoxy needs to test the version and supported extensions before dispatching a function, as it takes care of handling the different API:

Knows about function aliases so (e.g.) glBufferData() can be used with
GL_ARB_vertex_buffer_object implementations, along with GL 1.5+
implementations.

We cannot really cache the version and extension checks once performed, because you could call the same function using difference contexts; we could find a way to cache things a bit better, though.

On the other hand, querying the version and extensions should return static data and it's typically not a synchronization point: it should not affect performance in a drastic way. Are you enabling some debugging state?

@ebassi
Copy link
Collaborator

ebassi commented May 8, 2018

I do wonder, though, if you're using OpenGL < 3.2 and the old fixed function pipeline — and you're calling glGetString() between glBegin() and glEnd() pairs with some sort of debugging enabled.

Can you create a small, self-contained example?

@nwnk
Copy link
Collaborator

nwnk commented May 8, 2018

This looks more like the resolver is being called every time. As if the variable holding the function pointer wasn't being updated.

nwnk added a commit to nwnk/libepoxy that referenced this issue May 15, 2018
Our caller may load (eg) epoxy_glAlphaFunc, which is a function pointer,
and then call through that value multiple times. Until the caller
re-examines the value of that function pointer, which is a copy
relocation in the executable, repeated calls mean repeated work
resolving the GL function.

We can't make the caller reinspect the variable, but the resolver
function can avoid doing redundant work.

Fixes: anholt#171
Signed-off-by: Adam Jackson <ajax@redhat.com>
@nwnk
Copy link
Collaborator

nwnk commented May 15, 2018

@thrimbor if you could test the patch in #173 that'd be awesome.

@thrimbor
Copy link
Author

Sorry for not replying earlier, didn't have much time to spend on this in the last few days.

@nwnk That patch works very well, performance is now almost identical to not using libepoxy!

@ebassi The code can be configured to use OpenGL 1.1 or 2.1, but it uses glVertexPointer and friends instead of glBegin/glEnd. My reason for using libepoxy was to get rid of the manual function pointer handling for shaders and compressed textures. Afaik the code doesn't use any special debugging stuff.

@nwnk
Copy link
Collaborator

nwnk commented May 16, 2018

@thrimbor well I'm glad it helps, but I'm concerned that it'd be needed. Your code doesn't look like it's trying to save function addresses, so I would naively think that calling glVertexAttribArray on a second function entry would see the updated value of epoxy_glVertexAttribArray.

I wonder if this is a funny interaction with linking with -Bsymbolic, if our resolver is updating the value of the copy of the function pointer slot in libepoxy as opposed to the copy relocation slot in the main executable.

@nwnk
Copy link
Collaborator

nwnk commented May 16, 2018

@thrimbor could you test a build of libepoxy without the patch from #173, but with the -Bsymbolics in src/meson.build changed to -Bsymbolic-functions? I think that should make epoxy bind its function calls to itself but still use the copy relocs for data.

@thrimbor
Copy link
Author

@nwnk Changing that has the same positive effect on performance.

ebassi added a commit that referenced this issue May 17, 2018
Epoxy updates the function pointers in order to avoid calling the
resolver multiple times, but with -Bsymbolic we're going to update the
copy inside libepoxy, instead of the relocated copy in the code using
libepoxy. This leads to libepoxy constantly querying the function
resolver code instead of just once.

We still want to avoid intra-library relocations for our functions,
but we need to live with them for our global function pointers.

See issue #171
@ebassi
Copy link
Collaborator

ebassi commented May 17, 2018

Opened #174 with the linker flag change only, but given the testing, we should likely merge both.

nwnk pushed a commit that referenced this issue May 17, 2018
Epoxy updates the function pointers in order to avoid calling the
resolver multiple times, but with -Bsymbolic we're going to update the
copy inside libepoxy, instead of the relocated copy in the code using
libepoxy. This leads to libepoxy constantly querying the function
resolver code instead of just once.

We still want to avoid intra-library relocations for our functions,
but we need to live with them for our global function pointers.

See issue #171
@nwnk nwnk closed this as completed in #173 May 17, 2018
nwnk added a commit that referenced this issue May 17, 2018
Our caller may load (eg) epoxy_glAlphaFunc, which is a function pointer,
and then call through that value multiple times. Until the caller
re-examines the value of that function pointer, which is a copy
relocation in the executable, repeated calls mean repeated work
resolving the GL function.

We can't make the caller reinspect the variable, but the resolver
function can avoid doing redundant work.

Fixes: #171
Signed-off-by: Adam Jackson <ajax@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants