[PATCH nginx v2] remove ngx_cpuinfo, use sysconf or define

[Joe Konno]

Scroll to bottom (removing the v2-specific patch text and review as it's 
not relevant to my v1 commentary):

On 1/16/20 7:46 AM, Vladimir Homutov wrote:

[snip]

> On Thu, Jan 16, 2020 at 07:39:07AM -0800, Joe Konno wrote:

[snip]

>> v1 of that patch used CPUID to retrieve the cache line size directly
>> from H/W:
>>
>> http://mailman.nginx.org/pipermail/nginx-devel/2019-December/012853.html
> 
> yes, I've seen this too.

Thanks for the review!

> Although our current ngx_cpuinfo() may seem a bit outdated, it does its
> job, and contains some manually crafted cases. What we don't want is to
> make things more complex here and introduce more very specific code.

Fair enough. At the end of the day, nginx maintainers have to do what 
they feel is best for nginx.

Generally speaking, for any code base that wishes to optimize for, e.g., 
cache line size, the standing recommendation is to query the x86 
hardware directly by using the appropriate CPUID leaves.

Regardless of what happens to the patch(es), I wanted to spotlight CPUID 
and how it could help nginx-- this is just one case.

> If you know about specific cases when things get wrong with current
> implementation, please report.

Succinctly, no, I haven't seen things go wrong. I haven't done extensive 
testing, though.

Assuming the implementations of cache line-sensitive algorithms are 
correct (e.g. CRC), the case where nginx's code sets the wrong cache 
line size would probably yield sub-optimal performance on x86 hardware.

Further analysis would be needed to determine the exact cost of an 
incorrect cache line size on x86, given these implementations and 
how-often/when/where they are used.