[PATCH] Add observation that ports are not thread-safe

[PATCH] Add observation that ports are not thread-safe

[Diogo F. S. Ramos]

It's not obvious that ports are not thread-safe and trying to have
multiple threads writing to one returns errors that are not
recognizable as been caused by this lack of thread-safeness.

* doc/ref/guile.texi: non thread-safe observation
---
 doc/ref/api-io.texi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/doc/ref/api-io.texi b/doc/ref/api-io.texi
index 02d92a2..e9f79ae 100644
--- a/doc/ref/api-io.texi
+++ b/doc/ref/api-io.texi
@@ -88,6 +88,10 @@ representation for output. There are three possible strategies: to
 raise an error, to replace the character with a hex escape, or to
 replace the character with a substitute character.
 
+Ports are not thread-safe.  A port has to be protected if you want to
+access it from multiple threads using mechanisms like mutexes.
+(@pxref{Mutexes and Condition Variables})
+
 @rnindex input-port?
 @deffn {Scheme Procedure} input-port? x
 @deffnx {C Function} scm_input_port_p (x)
-- 
1.9.1

Re: [PATCH] Add observation that ports are not thread-safe

[Andy Wingo]

On Tue 25 Mar 2014 12:14, "Diogo F. S. Ramos" <dfsr@riseup.net> writes:

> It's not obvious that ports are not thread-safe and trying to have
> multiple threads writing to one returns errors that are not
> recognizable as been caused by this lack of thread-safeness.

This is a bug, and it is fixed in master.  FWIW.

Andy
-- 
http://wingolog.org/

Re: [PATCH] Add observation that ports are not thread-safe

[Diogo F. S. Ramos]

> On Tue 25 Mar 2014 12:14, "Diogo F. S. Ramos" <dfsr@riseup.net> writes:
>
>> It's not obvious that ports are not thread-safe and trying to have
>> multiple threads writing to one returns errors that are not
>> recognizable as been caused by this lack of thread-safeness.
>
> This is a bug, and it is fixed in master.  FWIW.

oic

This is an interesting issue between master and stable.

Browsing through the history, I see master's commit
79657fd3ec264ecd533a62d349c2cf1a2be2df14, which I assume fixes this bug,
but this fix is not in the last stable version, released just some days
ago.

IMO the user should be informed about this limitation in stable, even if
it's going to go away.

Re: [PATCH] Add observation that ports are not thread-safe

[Andy Wingo]

On Tue 25 Mar 2014 18:10, "Diogo F. S. Ramos" <dfsr@riseup.net> writes:

>> On Tue 25 Mar 2014 12:14, "Diogo F. S. Ramos" <dfsr@riseup.net> writes:
>>
>>> It's not obvious that ports are not thread-safe and trying to have
>>> multiple threads writing to one returns errors that are not
>>> recognizable as been caused by this lack of thread-safeness.
>>
>> This is a bug, and it is fixed in master.  FWIW.
>
> oic
>
> This is an interesting issue between master and stable.

Master is what will be 2.2.  stable-2.0 is the 2.0.x series.  Your point
is well taken, that ports aren't threadsafe in 2.0; but it pains me to
document a bug instead of fixing it ;-)

> IMO the user should be informed about this limitation in stable, even if
> it's going to go away.

Sure.  Would you mind posting a modified patch to this effect, noting
that this situation is fixed in 2.2?

Andy
-- 
http://wingolog.org/

[PATCH] Add observation that ports are not thread-safe

[Diogo F. S. Ramos]

It's not obvious that ports are not thread-safe and trying to have
multiple threads writing to one returns errors that are not
recognizable as been caused by this lack of thread-safeness.

* doc/ref/guile.texi: non thread-safe observation
---

I'm not sure about the verb tense of "Ports will be thread-safe in Guile
2.2." because you said "[...] this situation is fixed in 2.2."

 doc/ref/api-io.texi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/doc/ref/api-io.texi b/doc/ref/api-io.texi
index 02d92a2..de4020a 100644
--- a/doc/ref/api-io.texi
+++ b/doc/ref/api-io.texi
@@ -88,6 +88,11 @@ representation for output. There are three possible strategies: to
 raise an error, to replace the character with a hex escape, or to
 replace the character with a substitute character.
 
+Ports are not thread-safe.  A port has to be protected if you want to
+access it from multiple threads using mechanisms like mutexes
+(@pxref{Mutexes and Condition Variables}).  Ports will be thread-safe in
+Guile 2.2.
+
 @rnindex input-port?
 @deffn {Scheme Procedure} input-port? x
 @deffnx {C Function} scm_input_port_p (x)
-- 
1.9.1

Re: [PATCH] Add observation that ports are not thread-safe

[Diogo F. S. Ramos]

Until we resolve the issue if ports should be locked or not in the
future, I still think it's important to mention that ports are not
thread-safe in the manual.

Guile's I/O procedures should *not* do thread synchronization

[Mark H Weaver]

Andy Wingo <wingo@pobox.com> writes:

> On Tue 25 Mar 2014 12:14, "Diogo F. S. Ramos" <dfsr@riseup.net> writes:
>
>> It's not obvious that ports are not thread-safe and trying to have
>> multiple threads writing to one returns errors that are not
>> recognizable as been caused by this lack of thread-safeness.
>
> This is a bug, and it is fixed in master.  FWIW.

FWIW, I disagree that this is a bug.  I continue to believe that it
would be a very serious mistake to promise to do thread synchronization
within Guile's standard I/O procedures.

Standard Scheme programs that do their own parsing and printing using
'read-char', 'peek-char' and 'write-char' could be made at least an
order of magnitude faster in the future if we don't make this promise.

This could be done in a future version of Guile by uniformly using a
fixed encoding (UTF-8 or maybe UTF-32) for the port buffers of textual
ports, and doing the coding conversion when the buffer is filled or
flushed.

However, if we promise to do thread synchronization, we will condemn
Guile to forever having dog slow 'read-char', 'peek-char', 'write-char',
'get-u8', 'peek-u8', and 'put-u8' operations.

Consider string ports, for example.  They could be the basis for a very
natural and efficient method of writing string operations, especially
when we move to UTF-8 encoding of strings internally and string indexing
becomes less efficient, but only if we have fast single-character I/O.

In Guile's own internal read and print procedures, a global find-replace
was done in master to change all of these simple port operations to use
unlocked variants, presumably because it would have caused a major
performance regression otherwise.  What's the plan for portable Scheme
code that implements similar functionality, such as Oleg's libraries?

Most ports (by far) are only accessed by a single thread, but everyone
would have to pay the hefty price of built-in thread synchronization
whether they need it or not.

Finally, robust programs will have to do their own explicit
synchronization anyway.  Multiple threads writing to the same port
without explicit synchronization would lead to garbled output that is
interleaved at unspecified points.  The situation is even worse on the
read side.

In order to do proper I/O on the same port from multiple threads, the
locking _must_ be done within code that understands the meaning of the
data being read or written, because only such code can know where the
data can be interleaved without producing garbage.

For all of these reasons, I would strongly urge us to reconsider the
decision to promise internal locking for Guile's I/O procedures.

   Regards,
     Mark

Re: Guile's I/O procedures should *not* do thread synchronization

[Andy Wingo]

On Wed 26 Mar 2014 06:10, Mark H Weaver <mhw@netris.org> writes:

> Andy Wingo <wingo@pobox.com> writes:
n>
>> On Tue 25 Mar 2014 12:14, "Diogo F. S. Ramos" <dfsr@riseup.net> writes:
>>
>>> It's not obvious that ports are not thread-safe and trying to have
>>> multiple threads writing to one returns errors that are not
>>> recognizable as been caused by this lack of thread-safeness.
>>
>> This is a bug, and it is fixed in master.  FWIW.
>
> FWIW, I disagree that this is a bug.  I continue to believe that it
> would be a very serious mistake to promise to do thread synchronization
> within Guile's standard I/O procedures.

It seems to work for glibc streams.  Why do you think that thread
synchronization is inappropriate for Guile if it works for glibc?  If
you need more speed you can disable thread synchronization.  (Admittedly
there is no API for that yet, but the possibility exists, as in libc.)

> This could be done in a future version of Guile by uniformly using a
> fixed encoding (UTF-8 or maybe UTF-32) for the port buffers of textual
> ports, and doing the coding conversion when the buffer is filled or
> flushed.

We can do this already.

> However, if we promise to do thread synchronization, we will condemn
> Guile to forever having dog slow 'read-char', 'peek-char', 'write-char',
> 'get-u8', 'peek-u8', and 'put-u8' operations.

I think you are wrong about "dog slow".  Uncontended mutexes are fast,
and we can disable mutexen entirely for certain ports.

> Consider string ports, for example.  They could be the basis for a very
> natural and efficient method of writing string operations, especially
> when we move to UTF-8 encoding of strings internally and string indexing
> becomes less efficient, but only if we have fast single-character I/O.

In the case of SRFI-13-style procedures with fresh string output ports,
we can avoid synchronization.  For input ports, synchronization is
really cheap as you don't ever have to rebuffer.

In summary, I think this is a non-issue.

Regards,

Andy
-- 
http://wingolog.org/

Re: Guile's I/O procedures should *not* do thread synchronization

[Mark H Weaver]

Andy Wingo <wingo@pobox.com> writes:

> On Wed 26 Mar 2014 06:10, Mark H Weaver <mhw@netris.org> writes:
>
>> Andy Wingo <wingo@pobox.com> writes:
> n>
>>> On Tue 25 Mar 2014 12:14, "Diogo F. S. Ramos" <dfsr@riseup.net> writes:
>>>
>>>> It's not obvious that ports are not thread-safe and trying to have
>>>> multiple threads writing to one returns errors that are not
>>>> recognizable as been caused by this lack of thread-safeness.
>>>
>>> This is a bug, and it is fixed in master.  FWIW.
>>
>> FWIW, I disagree that this is a bug.  I continue to believe that it
>> would be a very serious mistake to promise to do thread synchronization
>> within Guile's standard I/O procedures.
>
> It seems to work for glibc streams.  Why do you think that thread
> synchronization is inappropriate for Guile if it works for glibc?

glibc implements the POSIX API, which (1) mandates that the I/O
functions do thread synchronization, and (2) provides standard
alternatives that avoid thread synchronization.

In the Scheme world, things are very different.  The Scheme standards
provide only one set of I/O primitives, and do not mandate that they do
thread synchronization.

>> However, if we promise to do thread synchronization, we will condemn
>> Guile to forever having dog slow 'read-char', 'peek-char', 'write-char',
>> 'get-u8', 'peek-u8', and 'put-u8' operations.
>
> I think you are wrong about "dog slow".  Uncontended mutexes are fast,

I did some benchmarks of 'putchar' vs 'putchar_unlocked' in C, without
contention.  I think it's fair to assume that the GCC and GLIBC folks
did a reasonably good job of making both of these as fast as they could.

Here's my test program:

--8<---------------cut here---------------start------------->8---
#include <stdio.h>

int
main (int argc, char *argv[])
{
  int i = 100000000;
  char ch = 'a';

  while (i--)
    {
      putchar (ch);
      if (ch == 'z')
        ch = 'a';
      else
        ch++;
    }
}
--8<---------------cut here---------------end--------------->8---

With gcc -O2, I tested two variants of this program: one with 'putchar'
and one with 'putchar_unlocked'.  On my YeeLoong (mips64el w/ N32 ABI),
the 'putchar_unlocked' version is faster by a factor of 26.3.

> and we can disable mutexen entirely for certain ports.

What set of ports would you suggest?  What about when portable programs
do I/O on stdin and stdout?

Since you ignored my strongest point, I'll repeat it:

  Finally, robust programs will have to do their own explicit
  synchronization anyway.  Multiple threads writing to the same port
  without explicit synchronization would lead to garbled output that is
  interleaved at unspecified points.  The situation is even worse on the
  read side.

  In order to do proper I/O on the same port from multiple threads, the
  locking _must_ be done within code that understands the meaning of the
  data being read or written, because only such code can know where the
  data can be interleaved without producing garbage.

     Regards,
       Mark

Re: Guile's I/O procedures should *not* do thread synchronization

[Andy Wingo]

Hi,

On Wed 26 Mar 2014 16:32, Mark H Weaver <mhw@netris.org> writes:

> Andy Wingo <wingo@pobox.com> writes:
>
>> It seems to work for glibc streams.  Why do you think that thread
>> synchronization is inappropriate for Guile if it works for glibc?
>
> In the Scheme world, things are very different.  The Scheme standards
> provide only one set of I/O primitives, and do not mandate that they do
> thread synchronization.

Do you think they should crash when used from two threads at once?  I
don't think that this acceptable for *any* Guile data structure.

>>> However, if we promise to do thread synchronization, we will condemn
>>> Guile to forever having dog slow 'read-char', 'peek-char', 'write-char',
>>> 'get-u8', 'peek-u8', and 'put-u8' operations.
>>
>> I think you are wrong about "dog slow".  Uncontended mutexes are fast,
>
> I did some benchmarks of 'putchar' vs 'putchar_unlocked' in C, without
> contention.  I think it's fair to assume that the GCC and GLIBC folks
> did a reasonably good job of making both of these as fast as they could.
>
> With gcc -O2, I tested two variants of this program: one with 'putchar'
> and one with 'putchar_unlocked'.  On my YeeLoong (mips64el w/ N32 ABI),
> the 'putchar_unlocked' version is faster by a factor of 26.3.

On my i7-2620M, the difference is only a factor of 3.0.

Now I think I understand your perspective; 26x is terrible.  But surely
this is an architecture problem, and not a Guile problem?  The world
will only get more multithreaded, and ignoring that does no one any
service.

>   Finally, robust programs will have to do their own explicit
>   synchronization anyway.  Multiple threads writing to the same port
>   without explicit synchronization would lead to garbled output that is
>   interleaved at unspecified points.  The situation is even worse on the
>   read side.
>
>   In order to do proper I/O on the same port from multiple threads, the
>   locking _must_ be done within code that understands the meaning of the
>   data being read or written, because only such code can know where the
>   data can be interleaved without producing garbage.

This is a good point but not germane to the crashing issue.

Andy
-- 
http://wingolog.org/

Re: Guile's I/O procedures should *not* do thread synchronization

[Mark H Weaver]

Andy Wingo <wingo@pobox.com> writes:

>>>> However, if we promise to do thread synchronization, we will condemn
>>>> Guile to forever having dog slow 'read-char', 'peek-char', 'write-char',
>>>> 'get-u8', 'peek-u8', and 'put-u8' operations.
>>>
>>> I think you are wrong about "dog slow".  Uncontended mutexes are fast,
>>
>> I did some benchmarks of 'putchar' vs 'putchar_unlocked' in C, without
>> contention.  I think it's fair to assume that the GCC and GLIBC folks
>> did a reasonably good job of making both of these as fast as they could.
>>
>> With gcc -O2, I tested two variants of this program: one with 'putchar'
>> and one with 'putchar_unlocked'.  On my YeeLoong (mips64el w/ N32 ABI),
>> the 'putchar_unlocked' version is faster by a factor of 26.3.
>
> On my i7-2620M, the difference is only a factor of 3.0.
>
> Now I think I understand your perspective; 26x is terrible.  But surely
> this is an architecture problem, and not a Guile problem?  The world
> will only get more multithreaded, and ignoring that does no one any
> service.

Now that I have access to the GCC Compile Farm, I repeated these
benchmarks on a variety of machines, and here are the results:

Ratio  CPU
=======================
26.3   Loongson 2F
14.0   PowerPC (Power7)
13.7   Loongson 3A
 9.33  ARMv6l
 6.47  UltraSparc IIe
 5.09  AMD Athlon II
 4.27  AMD Opteron
 3.46  Core 2 Duo P8600
=======================

     Mark

Re: Guile's I/O procedures should *not* do thread synchronization

[Ludovic Courtès]

Mark H Weaver <mhw@netris.org> skribis:

> Andy Wingo <wingo@pobox.com> writes:
>
>>>>> However, if we promise to do thread synchronization, we will condemn
>>>>> Guile to forever having dog slow 'read-char', 'peek-char', 'write-char',
>>>>> 'get-u8', 'peek-u8', and 'put-u8' operations.
>>>>
>>>> I think you are wrong about "dog slow".  Uncontended mutexes are fast,
>>>
>>> I did some benchmarks of 'putchar' vs 'putchar_unlocked' in C, without
>>> contention.  I think it's fair to assume that the GCC and GLIBC folks
>>> did a reasonably good job of making both of these as fast as they could.
>>>
>>> With gcc -O2, I tested two variants of this program: one with 'putchar'
>>> and one with 'putchar_unlocked'.  On my YeeLoong (mips64el w/ N32 ABI),
>>> the 'putchar_unlocked' version is faster by a factor of 26.3.
>>
>> On my i7-2620M, the difference is only a factor of 3.0.
>>
>> Now I think I understand your perspective; 26x is terrible.  But surely
>> this is an architecture problem, and not a Guile problem?  The world
>> will only get more multithreaded, and ignoring that does no one any
>> service.
>
> Now that I have access to the GCC Compile Farm, I repeated these
> benchmarks on a variety of machines, and here are the results:
>
> Ratio  CPU
> =======================
> 26.3   Loongson 2F
> 14.0   PowerPC (Power7)
> 13.7   Loongson 3A
>  9.33  ARMv6l
>  6.47  UltraSparc IIe
>  5.09  AMD Athlon II
>  4.27  AMD Opteron
>  3.46  Core 2 Duo P8600
> =======================

Interesting; I didn’t expect such differences between unlocked/locked,
and among architectures.

Ludo’.