Hi David,

On Sun, Jan 15, 2023 at 04:32:05PM +0000, David CARLIER wrote:
> Hi,
> 
> here a patch to disable the cpu affinity feature on macOs arm64,
> see the comments for more explanation.
> 
> Cheers !


> From 70283dc8295a17254dcb6772247f68c68160d708 Mon Sep 17 00:00:00 2001
> From: David CARLIER <devne...@gmail.com>
> Date: Sun, 15 Jan 2023 16:26:45 +0000
> Subject: [PATCH] BUILD: thread: disable cpu affinity on macOs arm64
> 
> The actual api, while still being available on this platform, is not
> working purposely as the concept of cpus had been reworked since the M1
> on arm64 thus only the QoS api is available to have a control over the
> cores.

Thanks, but really we shouldn't do that, for several reasons:

  - first, maybe it works for an older version of the OS and doing
    this will just break for some users ;

  - second, by doing this we prevent any future OS version from
    working out of the box

  - third, we're masking a predefined build option, which will
    just render a feature inoperant while appearing as enabled,
    and this is worse than throwing an error. And I really don't
    like the fact that the feature is enabled on the command line,
    reported as enabled in some C files and disabled in this one.
    
What I'd rather do is handle the error, check for MacOS and give a
hint to the user such as "Using MacOS ? Setting affinity on this OS
was broken since version X.Y, sorry", which will save users from
trying hard to get it to work and reporting issues. Also you don't
know, maybe we'll eventually find some shared libs to preload that
will provide compatibility with the old API for existing programs.

And in any case it's not a build issue, it's at minima a bug, possibly
an unfixable one, but seen from the user it's a bug as it's an advertised
feature that doesn't work.

Maybe there are other options, I don't know, but we must not address
it like this. Also we need to think about stable branches and what to
do for them (probably the same as for -dev).

Thanks!
Willy

Reply via email to