Re: [PATCH]: MEDIUM: enabling threads on osx
On Fri, Sep 13, 2019 at 05:17:59AM +0100, David CARLIER wrote: > Hi ok fair points I forgot the haproxy's policy for more targeted changes :-). > For the the thread_info it is both, as it is a pointer type in macOS. But if it's a pointer type it doesn't have any reason to clash with a symbol. I'm insisting because I find it awkward and surprising to have to prefix all variables in a project by the project's name itself to avoid clashes with anything else. I can understand this for types as this is quite common, but libraries rarely declare global symbols in a way that can clash with applications. Thanks, Willy
Re: [PATCH]: MEDIUM: enabling threads on osx
Hi ok fair points I forgot the haproxy's policy for more targeted changes :-). For the the thread_info it is both, as it is a pointer type in macOS. Here 3 patchset. Regards. On Fri, 13 Sep 2019 at 04:03, Willy Tarreau wrote: > > Hi David, > > On Sun, Sep 08, 2019 at 04:48:54PM +0100, David CARLIER wrote: > > Hi, > > > > Another little patch for improving osx support. > > Well, you should split your patch as it does at least 3 different things: > - rename thread_info to ha_thread_info (it's not even clear if it's the > type or the variable that causes conflict, thus please mention it in > the commit message). If it's only the type, as I expect, it would be > nice if we can avoid renaming the variable everywhere. > > - enable USE_CPU_AFFINITY for OSX. Please try to improve your change > using "#if defined(__APPLE__)" instead of "#if !defined(__APPLE__)" > as the construct looks quite confusing with nested #ifs. > > - enable USE_THREAD for OSX > > Otherwise I think the changes are OK, thanks for doing this! > Willy 0003-BUILD-MEDIUM-threads-enable-cpu_affinity-on-osx.patch Description: Binary data 0002-BUILD-MEDIUM-threads-refactoring-thread_info-struct.patch Description: Binary data 0001-BUILD-SMALL-threads-enable-threads-on-osx.patch Description: Binary data
Re: [PATCH]: MEDIUM: enabling threads on osx
On Fri, Sep 13, 2019 at 05:03:34AM +0200, Willy Tarreau wrote: > - enable USE_CPU_AFFINITY for OSX. Please try to improve your change > using "#if defined(__APPLE__)" instead of "#if !defined(__APPLE__)" > as the construct looks quite confusing with nested #ifs. On this last point, if the set_affinity code becomes really too messy, probably that we should have a wrapper in compat.h to do this per-os. Willy
Re: [PATCH]: MEDIUM: enabling threads on osx
Hi David, On Sun, Sep 08, 2019 at 04:48:54PM +0100, David CARLIER wrote: > Hi, > > Another little patch for improving osx support. Well, you should split your patch as it does at least 3 different things: - rename thread_info to ha_thread_info (it's not even clear if it's the type or the variable that causes conflict, thus please mention it in the commit message). If it's only the type, as I expect, it would be nice if we can avoid renaming the variable everywhere. - enable USE_CPU_AFFINITY for OSX. Please try to improve your change using "#if defined(__APPLE__)" instead of "#if !defined(__APPLE__)" as the construct looks quite confusing with nested #ifs. - enable USE_THREAD for OSX Otherwise I think the changes are OK, thanks for doing this! Willy
