public inbox for cygwin@cygwin.com
 help / color / mirror / Atom feed
From: Erik Bray <erik.m.bray@gmail.com>
To: cygwin@cygwin.com
Subject: Re: OpenBLAS patch for Cygwin
Date: Tue, 13 Feb 2018 18:23:00 -0000	[thread overview]
Message-ID: <CAOTD34ZfTkcrYd8JYwiOSMciQhDwF88oAogGbT1RQZ+oWixKBA@mail.gmail.com> (raw)
In-Reply-To: <a00f8d46-aca4-7eac-22c7-e035e001253d@gmail.com>

Hi, Sorry for the non-response. Been traveling for the last week and
not really checking this e-mail much.

On Tue, Feb 6, 2018 at 3:15 PM, Marco Atzeri wrote:
> On 06/02/2018 13:10, Erik Bray wrote:
>>
>> Hello,
>>
>> Users/maintainers of OpenBLAS on Cygwin may be interested in this
>> patch to improve support for fork():
>> https://github.com/xianyi/OpenBLAS/pull/1450
>>
>> Assuming this looks good (feedback welcome) it might be nice to have
>> this patch included in the next release of the official OpenBLAS
>> package for Cygwin since its incompatibility with fork() has caused
>> problems in the past [1].
>>
>>
>> Thanks,
>> E
>>
>> [1] https://trac.sagemath.org/ticket/22822
>>
>
>
> Noted.

And I see you just made a new package release incorporating my patch--thanks!

> Do you have a test case to show the failure with current build ?

The upstream pull request included such a test (the test had existed
previously in the code base but was disabled and never re-enabled in
the course of porting to a different testing framework).

> Any reason to use OS_CYGWIN_NT and not __CYGWIN__ in the patch ?

Normally I would just use __CYGWIN__ but this is following the
convention used throughout the rest of the OpenBLAS codebase.  It's
actually kind of nice that they have a consistent naming scheme for
platform macros.

On Tue, Feb 6, 2018 at 3:24 PM, Corinna Vinschen
<corinna-cygwin@cygwin.com> wrote:
> Also, it should really use pthread functions and drop the notion that
> Cygwin is a Windows target.  Assuming you're running fork from another
> thread than the main thread, does it still work with native Windows
> threads?

I definitely agree in general. I'm going to experiment with a second
patch to just have it use pthreads on Cygwin. I can't think of any
reason it shouldn't work. Is there any particular extra overhead to
pthreads in Cygwin over using the native API directly?  AFAICT the
only slight additional overhead is in thread creation, but that's not
a problem.

I haven't tried forking from another thread. I don't think that should
make an enormous difference, but I'll make a test case to try that.


Best,
E

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

  parent reply	other threads:[~2018-02-13 18:23 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-06 12:10 Erik Bray
2018-02-06 14:15 ` Marco Atzeri
2018-02-06 14:24   ` Corinna Vinschen
2018-02-13 18:23   ` Erik Bray [this message]
2018-02-06 18:07 ` Achim Gratz
2018-02-13 18:26   ` Erik Bray

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAOTD34ZfTkcrYd8JYwiOSMciQhDwF88oAogGbT1RQZ+oWixKBA@mail.gmail.com \
    --to=erik.m.bray@gmail.com \
    --cc=cygwin@cygwin.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).