[Mesa-dev] Reducing get.c size (and get_es1.c and get_es2.c)

[Kristian Høgsberg]

On Fri, May 21, 2010 at 4:16 PM, Brian Paul <brianp at vmware.com> wrote:
> Roland Scheidegger wrote:
>>
>> On 11.05.2010 20:08, Kristian Høgsberg wrote:
>>>
>>> 2010/5/11 Brian Paul <brianp at vmware.com>:
>>>>
>>>> Kristian Høgsberg wrote:
>>>>>
>>>>> 2010/5/6 Kristian Høgsberg <krh at bitplanet.net>:
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> Ok, I suppose this is not the most pressing issue in mesa, but I was
>>>>>> toying with an idea of how to reduce get.c size and integrate
>>>>>> get_es1.c and get_es2.c and I had to try it out.  Of course it ended
>>>>>> up being a bigger project and took a couple of days, but in the end I
>>>>>> think it turned out to be a worthwhile effort.  The result is the two
>>>>>> patches on the get-optimagix branch in my personal mesa repo:
>>>>>>
>>>>>>  http://cgit.freedesktop.org/~krh/mesa/log/?h=get-optimagix
>>>>>
>>>>> I've updated the patch on the branch here, but I'm still not going to
>>>>> send it to the list (it's 500k and most of which deletes generated
>>>>> code).  But I've been going back and forth with Brian about the patch
>>>>> a few times and we found a few bugs and made the code a little faster.
>>>>>  Brian cooked up a small micro-benchmark for the glGet*v() functions
>>>>> and it turned out that the code was actually slower than the old
>>>>> generated code.  With a little tweaking of the table lookup and adding
>>>>> a couple of likely() annotations, I got the code from 30s to 23s,
>>>>> where the old code would take 15s.  That's still 17 million gets per
>>>>> second, so I doubt the new code is going to be a bottleneck.
>>>>
>>>> It looks like piglit's texunits test is failing, compared to
>>>> Mesa/master.
>>>>  It may be an issue with flush-current when getting the curren texture
>>>> coords.
>>>
>>> Fixed, didn't initialize 'unit' in find_custom_value().
>>>
>>>> I'd like to see comments on more of the internal functions, like
>>>> find_value() to describe the parameters in/out.
>>>
>>> Ok, done.
>>>
>>>> Also, could you replace the #ifdef DEBUG stuff with a separate #ifdef
>>>> GET_DEBUG or similar to avoid printing the hash table collision info all
>>>> the
>>>> time in debug builds?
>>>
>>> Sure, done.
>>>
>>>> There hasn't been any other feedback so I'd so go ahead and commit it to
>>>> master when these things are fixed.
>>>
>>> Done, and I updated the table with your recent feedback changes.
>>
>> Just a small nitpick, but I got buried under roughly 300 compiler
>> warnings (missing initializer). Now that looks pretty harmless, but I
>> didn't feel like fixing all 300 occasions in the table manually :-(.
>
> I'll take a crack at fixing that...

Thanks Brian, didn't realize this was gcc (or is it C99?) specific.

Kristian