Re: [dylan] Stream locking (was: Method combinations as a design pattern for locking)

Howdy,

On Wed, Dec 18, 2013 at 7:24 PM, Hannes Mehnert <[email protected]> wrote:

> there's a thread from the beginning of this year on the same topic.
> Not sure why this is brought up so often.
>

Given your quotes, I assume you mean August of this year. (Took me a while
to find it when I was looking at the wrong months.)

As for why it is brought up repeatedly, I assume that's because it is an
actual source of pain and unhappiness with the status quo.


> In that discussion, I appreciate Bruce Hoult's comment:
> > I think that, except in a very few cases such as stderr, multiple
> threads
> > writing to the same stream is at the least bad program design and
> probably
> > a bug.
>
> > I would make the default to be non-locked, but with detection and
> warning
> > of writing from multiple threads, if cheap to do. e.g. store and
> > compare the thread IDs or TLS pointers or something. Even the stack
> > pointer
> would
> > suffice, if you can get the stack base and limit for the current
> > thread (only has to be one once per non-locking stream, the first
> > time it's
> used).
>
> And while I'm happy with the current code, having to explicitly
> annotate streams which do not require a lock, I'd be ok to remove
> these by-default locks if there is a warning implemented as Bruce
> proposes above.


The usual issue is who's going to do that work?  You, Bruce Hoult, or
someone else are more than welcome to do it and submit a pull request for
evaluation.

I've done the work that pretty much everyone agrees needs to be done:
Default to unlocked streams.  (Even your quotes from Bruce Hoult back that
up.)

The current code in master doesn't guarantee correctness, isn't in line
with the actual documentation, and is generally agreed upon to be not the
desired strategy for locking.


> On 12/18/2013 02:59, Bruce Mitchener wrote:
> > Unfortunately, a couple of years ago, some locks were introduced to
> > the streams library and I suspect that that has led to some issues
> > over time. An example of that is that in the typical case of
> > creating a <string-stream> for printing some data to it, we have to
> > be sure that we create it unlocked (like in the date code in the
> > system library), otherwise it can become a visible overhead when
> > profiling.
>
> I consider these stream locks to be easy catches during profiling.
>
> I also do not understand why you want to remove (in your branch) both
> the init-value for the private-stream-lock-value slot of
>

You mean why I set it to #f instead of make(<recursive-lock>)? Since in my
branch streams default to unlocked, they don't need a lock. There's
actually a noticeable overhead from setting up that lock and the finalizer
associated with it.

There seems to be agreement that the default case should be unlocked
streams, except for standard-io. That means that the default case should be
how I have it in the branch.


> <basic-stream>, as well as the calls to with-stream-locked from within
> io? Does the test whether stream-lock is non-false (in lock-stream)
> lower the performance in a significant way?
>

They don't do anything to guarantee actual correctness.  This was discussed
in the August thread as well:

  https://lists.opendylan.org/pipermail/hackers/2013-August/006816.html
  https://lists.opendylan.org/pipermail/hackers/2013-August/006822.html

They also prevent someone from using a regular lock on their stream,
requiring instead that it be a recursive lock.

As you've just pointed out in another email, lock-stream and unlock-stream
are open generics as well.

The application using streams from multiple threads needs to handle their
own synchronization to be correct. Once they're doing that, then the usage
of stream locks within the IO library is a non-issue. It isn't needed.

Aren't there more serious performance bottlenecks in the streams library?
>

Sure, there probably are. This isn't all about performance though.  I also
fixed a major issue with the buffer size that has a clear impact on
machines with slow disks. On my SSD machines, I barely notice it at all.
(This fix is in master already for 2013.2 and has been tested to have the
desired improvement by the Gentoo maintainer who is cursed with slow disks.)

 - Bruce

_______________________________________________
hackers mailing list
[email protected]
https://lists.opendylan.org/mailman/listinfo/hackers