On Wed, Feb 28, 2024 at 01:24:06AM +, Piotr Sikora via nginx-devel wrote:
> # HG changeset patch
> # User Piotr Sikora
> # Date 1708977640 0
> # Mon Feb 26 20:00:40 2024 +
> # Branch patch015
> # Node ID f58bc1041ebca635517b919d58b49923bf24f76d
> # Parent 570e97dddeeddb79c71587aa8a10150b64404beb
Hello,
Note that a compile time cache line size was previously improved
in 8010:35afae4b3dff, to have a reasonable default of 64.
> macOS: detect cache line size at runtime.
I prefer not to introduce more ad-hoc prefixes in the log summary.
Something like moving the "macOS" part to the end should be fine.
>
> Notably, Apple Silicon CPUs have 128 byte cache line size,
> which is twice the default configured for generic aarch64.
>
> Signed-off-by: Piotr Sikora
>
> diff -r 570e97dddeed -r f58bc1041ebc src/os/unix/ngx_darwin_init.c
> --- a/src/os/unix/ngx_darwin_init.c Mon Feb 26 20:00:38 2024 +
> +++ b/src/os/unix/ngx_darwin_init.c Mon Feb 26 20:00:40 2024 +
> @@ -11,6 +11,7 @@
>
> charngx_darwin_kern_ostype[16];
> charngx_darwin_kern_osrelease[128];
> +int64_t ngx_darwin_hw_cachelinesize;
> int ngx_darwin_hw_ncpu;
> int ngx_darwin_kern_ipc_somaxconn;
> u_long ngx_darwin_net_inet_tcp_sendspace;
style: this breaks a perfect indentation of two spaces after type;
further, it appears to be unsorted by type; I'd put it after u_long
Regarding ngx_darwin_net_inet_tcp_sendspace, which is u_long here, it
appears to be inherited from ngx_freebsd_init.c unmodified in 0.7.7.
However, tcp_sendspace is sysctl-exported using integer OID format
in both XNU and FreeBSD modern versions. Restoring its type there,
back to before 673:b80f94fa2197, might further affect the sorting.
As originally introduced in the BSD kernel, tcp_sendspace was int.
It was changed to u_long in 4.3BSD-Tahoe by Mike Karels (together
with soreserve), though with introduction of the (old style) sysctl
interface in FreeBSD 2.0.5, it started to be exported as type int
(with appropriate XXX). It continued to be exported as int using
the new style, until its OID format was change to SYSCTL_ULONG in
FreeBSD 7.0 "to match the type of the variable they are exporting".
Apparently, this was the reason of updating
ngx_freebsd_net_inet_tcp_sendspace to u_long in nginx 0.3.58
(further unsorting declarations with regard to sysctls[] order):
|*) Bugfix: nginx could not run on 64-bit FreeBSD 7.0-CURRENT.
Later, the OID format was changed back to int in FreeBSD 9.2, as
part of the sysctl virtualization work, with the following reason:
|A long is not necessary as the TCP window is limited to 2**30
but ngx_freebsd_net_inet_tcp_sendspace remained u_long, apparently
to keep nginx running on older FreeBSD version.
In XNU, tcp_sendspace interface remained the same as imported from
around FreeBSD 4.x, until in xnu-1456.1.26 (Snow Leopard), its type
was corrected to u_int32_t, and its sysctl OID format was changed
to "unsigned int".
Anyway, ngx_darwin_net_inet_tcp_sendspace is not currently used in
the code, we can leave it for now. See the relevant discussion on
incomplete work here:
https://mailman.nginx.org/pipermail/nginx-devel/2023-May/PSFQLA6TJSL7B2RKJWL6ORA54IOPO5SM.html
> @@ -44,6 +45,10 @@
>
>
> sysctl_t sysctls[] = {
> +{ "hw.cachelinesize",
> + _darwin_hw_cachelinesize,
> + sizeof(ngx_darwin_hw_cachelinesize), 0 },
> +
JFTR, this is well sorted regarding the order of declarations,
but it will need to be adjusted after fixing the sorting above.
> { "hw.ncpu",
>_darwin_hw_ncpu,
>sizeof(ngx_darwin_hw_ncpu), 0 },
> @@ -155,6 +160,7 @@
> return NGX_ERROR;
> }
>
> +ngx_cacheline_size = ngx_darwin_hw_cachelinesize;
> ngx_ncpu = ngx_darwin_hw_ncpu;
>
> if (ngx_darwin_kern_ipc_somaxconn > 32767) {
> diff -r 570e97dddeed -r f58bc1041ebc src/os/unix/ngx_posix_init.c
> --- a/src/os/unix/ngx_posix_init.cMon Feb 26 20:00:38 2024 +
> +++ b/src/os/unix/ngx_posix_init.cMon Feb 26 20:00:40 2024 +
> @@ -51,7 +51,10 @@
> }
>
> ngx_pagesize = getpagesize();
> -ngx_cacheline_size = NGX_CPU_CACHE_LINE;
> +
> +if (ngx_cacheline_size == 0) {
> +ngx_cacheline_size = NGX_CPU_CACHE_LINE;
> +}
>
> for (n = ngx_pagesize; n >>= 1; ngx_pagesize_shift++) { /* void */ }
>
This makes the following slight update to the patch.
If you're okey with it, I will commit it then.
# HG changeset patch
# User Piotr Sikora
# Date 1710886608 -14400
# Wed Mar 20 02:16:48 2024 +0400
# Node ID d6fc8103a60d5fe2ca291a3ee943a93725df420c
# Parent e773dcc47d1004babe8383b73c0731a734b70ca4
Detect cache line size at runtime on macOS.
Notably, Apple Silicon CPUs have 128 byte cache line size,
which is twice the default configured for generic aarch64.
Signed-off-by: Piotr Sikora
diff --git a/src/os/unix/ngx_darwin_init.c b/src/os/unix/ngx_darwin_init.c
--- a/src/os/unix/ngx_darwin_init.c
+++
