unofficial mirror of guile-devel@gnu.org 
 help / color / mirror / Atom feed
* array handles and non-local exits
@ 2008-09-15 20:17 Neil Jerram
  2008-09-16  7:56 ` Ludovic Courtès
  0 siblings, 1 reply; 23+ messages in thread
From: Neil Jerram @ 2008-09-15 20:17 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel

2008/9/15 Ludovic Courtès <ludo@gnu.org>:
>
> "Neil Jerram" <neiljerram@googlemail.com> writes:
>
>> Now, as it happens, the code doesn't actually implement what the
>> manual says, and in fact scm_array_handle_release() is currently a
>> no-op!  But I believe the manual's intention is sensible, so it think
>> I think we should commit this patch as-is for now, and raise a bug to
>> track the fact that the array/handle API isn't fully implemented.
>>
>> What do you think?
>
> I'd prefer fixing the manual rather than `scm_array_get_handle ()',
> because (1) setting up a dynwind is "costly" (involves some consing),

Which implies that we should avoid it if not needed, which to me
implies that it makes sense to have it _inside_ the scm_array*
functions, because the implementation of those functions determines
whether it is needed.  (Currently it isn't needed, because the
functions don't actually perform any reservation.)

> and (2) I don't know of any other function that does a dynwind behind
> the scenes (IOW, let's not break the "rule of least surprise").

I think you're imagining a clear boundary here where there isn't one.
If needed, either the scm_dynwind would be inside
scm_array_get_handle, or it would be inside scm_uniform_vector_read.
Both of those are public libguile functions, so where's the
difference?

(It may also help to grep for scm_dynwind_begin, to see all current
uses; there are quite a few.)

> OTOH, it may be the case that people have been relied on the described
> behavior, in which case it would be wiser to fix `scm_array_get_handle ()'
> to conform to the manual...

Note that scm_array_get_handle can itself throw an error, and so can
the other APIs that return a handle to the caller.  That suggests to
me that the scm_dynwind* stuff might have to be implemented inside
these APIs - because otherwise it wouldn't be covering all the
possible non-local exits.

On the other hand, it might be that all of the error cases are just
wrong-type-args, and so don't really count.  I haven't checked them
all in detail yet.

One more observation: we should take care when adding occurrences of
scm_dynwind_begin (0) into code, because it prevents the enclosed code
from capturing its continuation, returning, and then calling the
continuation again later.  In this case, I wonder if there could be
practical soft port implementations that would want to do this.  (I
thought I had done it myself when writing (gui entry-port) for
guile-gui, but in fact that only uses a continuation as an escape
mechanism (i.e. for jumping back up the stack).)  On the other hand,
if we use scm_dynwind_begin (SCM_F_DYNWIND_REWINDABLE), to allow a
continuation to be called again from outside, that raises the question
of what happens if the array in question has changed since the
continuation was captured.

In summary, I think this is all quite tricky, and I don't have a good
answer yet!

Regards,
        Neil




^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: array handles and non-local exits
  2008-09-15 20:17 array handles and non-local exits Neil Jerram
@ 2008-09-16  7:56 ` Ludovic Courtès
  2008-09-17 19:32   ` Neil Jerram
  0 siblings, 1 reply; 23+ messages in thread
From: Ludovic Courtès @ 2008-09-16  7:56 UTC (permalink / raw)
  To: guile-devel

Hello!

"Neil Jerram" <neiljerram@googlemail.com> writes:

> 2008/9/15 Ludovic Courtès <ludo@gnu.org>:
>>
>> "Neil Jerram" <neiljerram@googlemail.com> writes:
>>
>>> Now, as it happens, the code doesn't actually implement what the
>>> manual says, and in fact scm_array_handle_release() is currently a
>>> no-op!  But I believe the manual's intention is sensible, so it think
>>> I think we should commit this patch as-is for now, and raise a bug to
>>> track the fact that the array/handle API isn't fully implemented.
>>>
>>> What do you think?
>>
>> I'd prefer fixing the manual rather than `scm_array_get_handle ()',
>> because (1) setting up a dynwind is "costly" (involves some consing),
>
> Which implies that we should avoid it if not needed, which to me
> implies that it makes sense to have it _inside_ the scm_array*
> functions, because the implementation of those functions determines
> whether it is needed.  (Currently it isn't needed, because the
> functions don't actually perform any reservation.)

If `get_handle ()' creates a dynwind that ends in `release_handle ()',
then code like the following will no longer work as expected:

  scm_dynwind_begin ();
  scm_dynwind_unwind_handler ();
  scm array_get_handle ();

  ...

  scm_dynwind_end ();
  scm_array_release_handle ();

Perhaps that's what is implied by the manual's wording but I find it a
bit tricky.

>> and (2) I don't know of any other function that does a dynwind behind
>> the scenes (IOW, let's not break the "rule of least surprise").

I meant "I don't know of a function that does a `dynwind_begin'
*alone*" (of course there are plenty of functions that do
`dynwind_begin' + `dynwind_end').

> I think you're imagining a clear boundary here where there isn't one.
> If needed, either the scm_dynwind would be inside
> scm_array_get_handle, or it would be inside scm_uniform_vector_read.
> Both of those are public libguile functions, so where's the
> difference?

The difference is that `scm_array_get_handle ()' is a low-level
function.  It may be used, say, in code that passes SRFI-4 vectors to C
that implements "performance-critical" code.  Adding consing in there
wouldn't feel right.

>> OTOH, it may be the case that people have been relied on the described
>> behavior, in which case it would be wiser to fix `scm_array_get_handle ()'
>> to conform to the manual...
>
> Note that scm_array_get_handle can itself throw an error, and so can
> the other APIs that return a handle to the caller.

That would suggest adding a `dynwind_begin'/`dynwind_end' pair, not a
`dynwind_begin' alone---but that is not actually needed, since none of
the callees of `scm_array_get_handle ()' can throw an error, except for
`scm_wrong_type_arg_msg ()' at the end.

> One more observation: we should take care when adding occurrences of
> scm_dynwind_begin (0) into code, because it prevents the enclosed code
> from capturing its continuation, returning, and then calling the
> continuation again later.  In this case, I wonder if there could be
> practical soft port implementations that would want to do this.  (I
> thought I had done it myself when writing (gui entry-port) for
> guile-gui, but in fact that only uses a continuation as an escape
> mechanism (i.e. for jumping back up the stack).)  On the other hand,
> if we use scm_dynwind_begin (SCM_F_DYNWIND_REWINDABLE), to allow a
> continuation to be called again from outside, that raises the question
> of what happens if the array in question has changed since the
> continuation was captured.

Right, I hadn't thought about it, but as you mention, a dynwind in
`uniform-vector-read!' will only affect soft port implementations.

Thanks,
Ludo'.





^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: array handles and non-local exits
  2008-09-16  7:56 ` Ludovic Courtès
@ 2008-09-17 19:32   ` Neil Jerram
  2008-09-18  8:15     ` Ludovic Courtès
  0 siblings, 1 reply; 23+ messages in thread
From: Neil Jerram @ 2008-09-17 19:32 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel

2008/9/16 Ludovic Courtès <ludo@gnu.org>:
>
> If `get_handle ()' creates a dynwind that ends in `release_handle ()',
> then code like the following will no longer work as expected:
>
>  scm_dynwind_begin ();
>  scm_dynwind_unwind_handler ();
>  scm array_get_handle ();
>
>  ...
>
>  scm_dynwind_end ();
>  scm_array_release_handle ();
>
> Perhaps that's what is implied by the manual's wording but I find it a
> bit tricky.

Yes, that definitely is what the manual means by "properly nested" -
but I also agree that it is a bit tricky.

>>> and (2) I don't know of any other function that does a dynwind behind
>>> the scenes (IOW, let's not break the "rule of least surprise").
>
> I meant "I don't know of a function that does a `dynwind_begin'
> *alone*" (of course there are plenty of functions that do
> `dynwind_begin' + `dynwind_end').

Yes, I see what you mean now.  (The scm_dynwind_begin() being in
scm_array_get_handle(), and the scm_dynwind_end() being in
scm_array_release_handle().)

>> I think you're imagining a clear boundary here where there isn't one.
>> If needed, either the scm_dynwind would be inside
>> scm_array_get_handle, or it would be inside scm_uniform_vector_read.
>> Both of those are public libguile functions, so where's the
>> difference?
>
> The difference is that `scm_array_get_handle ()' is a low-level
> function.  It may be used, say, in code that passes SRFI-4 vectors to C
> that implements "performance-critical" code.  Adding consing in there
> wouldn't feel right.

If you add in " and which can't possibly do a non-local exit " there,
I see your point.

> Right, I hadn't thought about it, but as you mention, a dynwind in
> `uniform-vector-read!' will only affect soft port implementations.

With that in mind, do you think we need to solve this now?  I think
this is low impact, so for now I'm inclined just to raise a bug in
Savannah, containing our discussion so far, so that we don't forget
it.

Regards,
        Neil




^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: array handles and non-local exits
  2008-09-17 19:32   ` Neil Jerram
@ 2008-09-18  8:15     ` Ludovic Courtès
  2008-09-18  9:17       ` Neil Jerram
  0 siblings, 1 reply; 23+ messages in thread
From: Ludovic Courtès @ 2008-09-18  8:15 UTC (permalink / raw)
  To: guile-devel

Hi,

"Neil Jerram" <neiljerram@googlemail.com> writes:

>>>> and (2) I don't know of any other function that does a dynwind behind
>>>> the scenes (IOW, let's not break the "rule of least surprise").
>>
>> I meant "I don't know of a function that does a `dynwind_begin'
>> *alone*" (of course there are plenty of functions that do
>> `dynwind_begin' + `dynwind_end').
>
> Yes, I see what you mean now.  (The scm_dynwind_begin() being in
> scm_array_get_handle(), and the scm_dynwind_end() being in
> scm_array_release_handle().)

Yes.

>>> I think you're imagining a clear boundary here where there isn't one.
>>> If needed, either the scm_dynwind would be inside
>>> scm_array_get_handle, or it would be inside scm_uniform_vector_read.
>>> Both of those are public libguile functions, so where's the
>>> difference?
>>
>> The difference is that `scm_array_get_handle ()' is a low-level
>> function.  It may be used, say, in code that passes SRFI-4 vectors to C
>> that implements "performance-critical" code.  Adding consing in there
>> wouldn't feel right.
>
> If you add in " and which can't possibly do a non-local exit " there,
> I see your point.

Exactly.

>> Right, I hadn't thought about it, but as you mention, a dynwind in
>> `uniform-vector-read!' will only affect soft port implementations.
>
> With that in mind, do you think we need to solve this now?  I think
> this is low impact, so for now I'm inclined just to raise a bug in
> Savannah, containing our discussion so far, so that we don't forget
> it.

I submitted this bug:

  https://savannah.gnu.org/bugs/index.php?24292

(Oops I forgot "and dynwinds" in the bug title...)

I would suggest that we drop that mention of dynwinds from the manual.

Thanks,
Ludo'.





^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: array handles and non-local exits
  2008-09-18  8:15     ` Ludovic Courtès
@ 2008-09-18  9:17       ` Neil Jerram
  2008-09-18 13:44         ` Ludovic Courtès
  2009-07-04 19:28         ` Andy Wingo
  0 siblings, 2 replies; 23+ messages in thread
From: Neil Jerram @ 2008-09-18  9:17 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel

2008/9/18 Ludovic Courtès <ludo@gnu.org>:
>
> I submitted this bug:
>
>  https://savannah.gnu.org/bugs/index.php?24292

That's great, thanks.

> I would suggest that we drop that mention of dynwinds from the manual.

In my view, this part is still useful:

"You must take care to always unreserve an array after reserving it,
also in the presence of non-local exits. To simplify this, reserving
and unreserving work like a dynwind context (see Dynamic Wind): a call
to scm_array_get_handle can be thought of as beginning a dynwind
context and scm_array_handle_release as ending it. When a non-local
exit happens between these two calls, the array is implicitely
unreserved.

That is, you need to properly pair reserving and unreserving in your
code, but you don't need to worry about non-local exits."

But this part is misleading and can be removed:

"These calls and other pairs of calls that establish dynwind contexts
need to be properly nested. If you begin a context prior to reserving
an array, you need to unreserve the array before ending the context.
Likewise, when reserving two or more arrays in a certain order, you
need to unreserve them in the opposite order."

Is that what you had in mind?

      Neil




^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: array handles and non-local exits
  2008-09-18  9:17       ` Neil Jerram
@ 2008-09-18 13:44         ` Ludovic Courtès
  2009-06-30 22:59           ` Neil Jerram
  2009-07-04 19:28         ` Andy Wingo
  1 sibling, 1 reply; 23+ messages in thread
From: Ludovic Courtès @ 2008-09-18 13:44 UTC (permalink / raw)
  To: guile-devel

"Neil Jerram" <neiljerram@googlemail.com> writes:

> 2008/9/18 Ludovic Courtès <ludo@gnu.org>:

>> I would suggest that we drop that mention of dynwinds from the manual.

Just to clarify: I'm suggesting fixing the manual so that it conforms to
the implementation, not the opposite.

> In my view, this part is still useful:
>
> "You must take care to always unreserve an array after reserving it,
> also in the presence of non-local exits. To simplify this, reserving
> and unreserving work like a dynwind context (see Dynamic Wind): a call
> to scm_array_get_handle can be thought of as beginning a dynwind
> context and scm_array_handle_release as ending it. When a non-local
> exit happens between these two calls, the array is implicitely
> unreserved.
>
> That is, you need to properly pair reserving and unreserving in your
> code, but you don't need to worry about non-local exits."
>
> But this part is misleading and can be removed:
>
> "These calls and other pairs of calls that establish dynwind contexts
> need to be properly nested. If you begin a context prior to reserving
> an array, you need to unreserve the array before ending the context.
> Likewise, when reserving two or more arrays in a certain order, you
> need to unreserve them in the opposite order."
>
> Is that what you had in mind?

Not quite.  If get/release are left as is (i.e., they do not establish a
dynwind context), I would write something along the lines of:

     You must take care to always unreserve an array after reserving it,
  also in the presence of non-local exits.  If a non-local exit can
  happen between these two calls, you should install a dynwind context
  that releases the array when it is left (see Dynamic Wind).

     In addition, array reserving and unreserving must be properly
  paired.  For instance, when reserving two or more arrays in a certain
  order, you need to unreserve them in the opposite order.

I would simply remove the following paragraph:

     These calls and other pairs of calls that establish dynwind contexts
  need to be properly nested.  If you begin a context prior to reserving
  an array, you need to unreserve the array before ending the context.
  Likewise, when reserving two or more arrays in a certain order, you
  need to unreserve them in the opposite order.

What do you think?

Thanks,
Ludo'.





^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: array handles and non-local exits
  2008-09-18 13:44         ` Ludovic Courtès
@ 2009-06-30 22:59           ` Neil Jerram
  2009-07-01  8:37             ` Ludovic Courtès
  0 siblings, 1 reply; 23+ messages in thread
From: Neil Jerram @ 2009-06-30 22:59 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel

ludo@gnu.org (Ludovic Courtès) writes:

> Not quite.  If get/release are left as is (i.e., they do not establish a
> dynwind context), I would write something along the lines of:
>
>      You must take care to always unreserve an array after reserving it,
>   also in the presence of non-local exits.  If a non-local exit can
>   happen between these two calls, you should install a dynwind context
>   that releases the array when it is left (see Dynamic Wind).
>
>      In addition, array reserving and unreserving must be properly
>   paired.  For instance, when reserving two or more arrays in a certain
>   order, you need to unreserve them in the opposite order.
>
> I would simply remove the following paragraph:
>
>      These calls and other pairs of calls that establish dynwind contexts
>   need to be properly nested.  If you begin a context prior to reserving
>   an array, you need to unreserve the array before ending the context.
>   Likewise, when reserving two or more arrays in a certain order, you
>   need to unreserve them in the opposite order.
>
> What do you think?

Another loose end...  I agree and have committed your suggested text
to branch_release-1-8 and master.

Regards,
        Neil




^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: array handles and non-local exits
  2009-06-30 22:59           ` Neil Jerram
@ 2009-07-01  8:37             ` Ludovic Courtès
  2009-07-01 22:04               ` Neil Jerram
  0 siblings, 1 reply; 23+ messages in thread
From: Ludovic Courtès @ 2009-07-01  8:37 UTC (permalink / raw)
  To: Neil Jerram; +Cc: guile-devel

Neil Jerram <neil@ossau.uklinux.net> writes:

> ludo@gnu.org (Ludovic Courtès) writes:
>
>> Not quite.  If get/release are left as is (i.e., they do not establish a
>> dynwind context), I would write something along the lines of:
>>
>>      You must take care to always unreserve an array after reserving it,
>>   also in the presence of non-local exits.  If a non-local exit can
>>   happen between these two calls, you should install a dynwind context
>>   that releases the array when it is left (see Dynamic Wind).
>>
>>      In addition, array reserving and unreserving must be properly
>>   paired.  For instance, when reserving two or more arrays in a certain
>>   order, you need to unreserve them in the opposite order.
>>
>> I would simply remove the following paragraph:
>>
>>      These calls and other pairs of calls that establish dynwind contexts
>>   need to be properly nested.  If you begin a context prior to reserving
>>   an array, you need to unreserve the array before ending the context.
>>   Likewise, when reserving two or more arrays in a certain order, you
>>   need to unreserve them in the opposite order.
>>
>> What do you think?
>
> Another loose end...  I agree and have committed your suggested text
> to branch_release-1-8 and master.

Cool, thanks for going over all these loose ends!  ;-)

Maybe 1.8.7 is approaching now?

Ludo'.




^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: array handles and non-local exits
  2009-07-01  8:37             ` Ludovic Courtès
@ 2009-07-01 22:04               ` Neil Jerram
  2009-07-01 22:36                 ` Ludovic Courtès
  0 siblings, 1 reply; 23+ messages in thread
From: Neil Jerram @ 2009-07-01 22:04 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel

ludo@gnu.org (Ludovic Courtès) writes:

> Cool, thanks for going over all these loose ends!  ;-)

I'm really happy to be getting through them... these are things that
have been sitting in my inbox for ages.

> Maybe 1.8.7 is approaching now?

Yes, that would be good.  It might just be worth fixing the jmp_buf
definition problem before that (which I should get to in the next week
or so) - but on the other hand we already have plenty of fixes for a
1.8.7.  When would be a good time for you to do a release?

(Assuming here that you don't mind doing the mechanics.  In case it
isn't convenient, I don't mind learning!)

Regards,
        Neil




^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: array handles and non-local exits
  2009-07-01 22:04               ` Neil Jerram
@ 2009-07-01 22:36                 ` Ludovic Courtès
  2009-07-02 23:33                   ` Neil Jerram
  0 siblings, 1 reply; 23+ messages in thread
From: Ludovic Courtès @ 2009-07-01 22:36 UTC (permalink / raw)
  To: guile-devel

Neil Jerram <neil@ossau.uklinux.net> writes:

> ludo@gnu.org (Ludovic Courtès) writes:

>> Maybe 1.8.7 is approaching now?
>
> Yes, that would be good.  It might just be worth fixing the jmp_buf
> definition problem before that (which I should get to in the next week
> or so) - but on the other hand we already have plenty of fixes for a
> 1.8.7.  When would be a good time for you to do a release?

Hmm, maybe by Sunday, 5th, which would leave me one release-free
week-end before 1.9.1.  :-)

> (Assuming here that you don't mind doing the mechanics.  In case it
> isn't convenient, I don't mind learning!)

That's OK for now.  Eventually, I might feel the need to change, at
which point I'll let you know!

Thank you,
Ludo'.





^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: array handles and non-local exits
  2009-07-01 22:36                 ` Ludovic Courtès
@ 2009-07-02 23:33                   ` Neil Jerram
  2009-07-03 23:38                     ` Neil Jerram
  0 siblings, 1 reply; 23+ messages in thread
From: Neil Jerram @ 2009-07-02 23:33 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel

ludo@gnu.org (Ludovic Courtès) writes:

> Neil Jerram <neil@ossau.uklinux.net> writes:
>
>> ludo@gnu.org (Ludovic Courtès) writes:
>
>>> Maybe 1.8.7 is approaching now?
>>
>> Yes, that would be good.  It might just be worth fixing the jmp_buf
>> definition problem before that (which I should get to in the next week
>> or so) - but on the other hand we already have plenty of fixes for a
>> 1.8.7.  When would be a good time for you to do a release?
>
> Hmm, maybe by Sunday, 5th, which would leave me one release-free
> week-end before 1.9.1.  :-)

Great!  I won't have time for jmp_buf before then, but I would like to
check that I haven't forgotten NEWS for the things I've worked on, and
I should be able to do that tomorrow evening or Saturday.

>> (Assuming here that you don't mind doing the mechanics.  In case it
>> isn't convenient, I don't mind learning!)
>
> That's OK for now.  Eventually, I might feel the need to change, at
> which point I'll let you know!

That sounds good to me.

Regards,
        Neil




^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: array handles and non-local exits
  2009-07-02 23:33                   ` Neil Jerram
@ 2009-07-03 23:38                     ` Neil Jerram
  0 siblings, 0 replies; 23+ messages in thread
From: Neil Jerram @ 2009-07-03 23:38 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel

Neil Jerram <neil@ossau.uklinux.net> writes:

> [..] but I would like to
> check that I haven't forgotten NEWS for the things I've worked on, and
> I should be able to do that tomorrow evening or Saturday.

I've done that now, resulting in [1], so please feel free to go ahead
with the release whenever it is convenient for you.

Regards,
        Neil

[1] http://git.savannah.gnu.org/gitweb/?p=guile.git;a=commit;h=9f6b657549280f0bd23e2f285d69370151d40549




^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: array handles and non-local exits
  2008-09-18  9:17       ` Neil Jerram
  2008-09-18 13:44         ` Ludovic Courtès
@ 2009-07-04 19:28         ` Andy Wingo
  2009-07-05 11:14           ` Ludovic Courtès
  1 sibling, 1 reply; 23+ messages in thread
From: Andy Wingo @ 2009-07-04 19:28 UTC (permalink / raw)
  To: Neil Jerram; +Cc: Ludovic Courtès, guile-devel

Hey folks :)

On Thu 18 Sep 2008 11:17, "Neil Jerram" <neiljerram@googlemail.com> writes:

> In my view, this part is still useful:
>
> "You must take care to always unreserve an array after reserving it,
> also in the presence of non-local exits. To simplify this, reserving
> and unreserving work like a dynwind context (see Dynamic Wind): a call
> to scm_array_get_handle can be thought of as beginning a dynwind
> context and scm_array_handle_release as ending it. When a non-local
> exit happens between these two calls, the array is implicitely
> unreserved.
>
> That is, you need to properly pair reserving and unreserving in your
> code, but you don't need to worry about non-local exits."

I'm hacking on this code right now, and have come to think that
scm_array_handle_release is superfluous. It shouldn't protect against
concurrent modification of the data, as that should be done with
user-implemented mutexen. It obviously doesn't do anything now -- and
really, it doesn't have anything to do. I can't think of a /supported/
modification of an array that will leave a pointer to the elements
invalid -- i.e. we have no truncation ops that I know of.

The pointer will always be valid as long as we have a reference to the
array itself (which we do, on the stack, in the scm_t_array_handle).

Andy
-- 
http://wingolog.org/




^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: array handles and non-local exits
  2009-07-04 19:28         ` Andy Wingo
@ 2009-07-05 11:14           ` Ludovic Courtès
  2009-07-06 14:09             ` Andy Wingo
  0 siblings, 1 reply; 23+ messages in thread
From: Ludovic Courtès @ 2009-07-05 11:14 UTC (permalink / raw)
  To: guile-devel

Hello,

Andy Wingo <wingo@pobox.com> writes:

> I'm hacking on this code right now, and have come to think that
> scm_array_handle_release is superfluous. It shouldn't protect against
> concurrent modification of the data, as that should be done with
> user-implemented mutexen. It obviously doesn't do anything now -- and
> really, it doesn't have anything to do. I can't think of a /supported/
> modification of an array that will leave a pointer to the elements
> invalid -- i.e. we have no truncation ops that I know of.

How about copy-on-write stringbufs as produced by `substring'?  I
suppose it's only a problem is the stringbuf is accessed concurrently,
though.

Thanks,
Ludo'.





^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: array handles and non-local exits
  2009-07-05 11:14           ` Ludovic Courtès
@ 2009-07-06 14:09             ` Andy Wingo
  2009-07-06 20:30               ` Ludovic Courtès
  0 siblings, 1 reply; 23+ messages in thread
From: Andy Wingo @ 2009-07-06 14:09 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel

Hi,

On Sun 05 Jul 2009 12:14, ludo@gnu.org (Ludovic Courtès) writes:

> Andy Wingo <wingo@pobox.com> writes:
>
>> I'm hacking on this code right now, and have come to think that
>> scm_array_handle_release is superfluous. It shouldn't protect against
>> concurrent modification of the data, as that should be done with
>> user-implemented mutexen. It obviously doesn't do anything now -- and
>> really, it doesn't have anything to do. I can't think of a /supported/
>> modification of an array that will leave a pointer to the elements
>> invalid -- i.e. we have no truncation ops that I know of.
>
> How about copy-on-write stringbufs as produced by `substring'?  I
> suppose it's only a problem is the stringbuf is accessed concurrently,
> though.

Good question. I suppose you have this case in mind:

  (define s0 (string "foooooo"))
  (define s1 (substring/shared s0 0))
  (define s2 (substring/shared s1 0))
  (par-for-each (lambda (s) (string-set! s 0 #\b))
                (list s0 s1 s2))

I suppose that's a case in which scm_array_handle_elements and
scm_array_handle_writable_elements should do different things. But once
you have the array handle, it doesn't seem that it needs to be released.
(The validity of the pointer should be assured by the reference that
each string has to its stringbuf.)

Andy
-- 
http://wingolog.org/




^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: array handles and non-local exits
  2009-07-06 14:09             ` Andy Wingo
@ 2009-07-06 20:30               ` Ludovic Courtès
  2009-07-09 19:19                 ` Andy Wingo
  0 siblings, 1 reply; 23+ messages in thread
From: Ludovic Courtès @ 2009-07-06 20:30 UTC (permalink / raw)
  To: guile-devel

Hello,

Andy Wingo <wingo@pobox.com> writes:

> Good question. I suppose you have this case in mind:
>
>   (define s0 (string "foooooo"))
>   (define s1 (substring/shared s0 0))
>   (define s2 (substring/shared s1 0))
>   (par-for-each (lambda (s) (string-set! s 0 #\b))
>                 (list s0 s1 s2))

Yes.  OTOH, the doc doesn't say that concurrent array accesses are safe,
so array accesses are supposed to be synchronized at the application
level, using mutexes, I suppose.

> I suppose that's a case in which scm_array_handle_elements and
> scm_array_handle_writable_elements should do different things. But once
> you have the array handle, it doesn't seem that it needs to be released.
> (The validity of the pointer should be assured by the reference that
> each string has to its stringbuf.)

Yes.

Still, I don't feel like we have any compelling reason to remove
`scm_array_handle_release ()'.  One argument to keep it is that it's the
kind of thing that's much easier to remove than to reinstate, and "we
never know".  Also, removing it would cause gratuitous incompatibility.

Thanks,
Ludo'.





^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: array handles and non-local exits
  2009-07-06 20:30               ` Ludovic Courtès
@ 2009-07-09 19:19                 ` Andy Wingo
  2009-07-09 21:08                   ` Ludovic Courtès
  0 siblings, 1 reply; 23+ messages in thread
From: Andy Wingo @ 2009-07-09 19:19 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel

Hi,

On Mon 06 Jul 2009 21:30, ludo@gnu.org (Ludovic Courtès) writes:

> Andy Wingo <wingo@pobox.com> writes:
>
>> Good question. I suppose you have this case in mind:
>>
>>   (define s0 (string "foooooo"))
>>   (define s1 (substring/shared s0 0))
>>   (define s2 (substring/shared s1 0))
>>   (par-for-each (lambda (s) (string-set! s 0 #\b))
>>                 (list s0 s1 s2))
>
> Yes.  OTOH, the doc doesn't say that concurrent array accesses are safe,
> so array accesses are supposed to be synchronized at the application
> level, using mutexes, I suppose.

They should be safe in the sense that they shouldn't crash Guile, but
the result may be strange -- e.g. hashtable insertion.

> Still, I don't feel like we have any compelling reason to remove
> `scm_array_handle_release ()'.  One argument to keep it is that it's the
> kind of thing that's much easier to remove than to reinstate, and "we
> never know".  Also, removing it would cause gratuitous
> incompatibility.

To me this is a weak argument, especially given that much code probably
doesn't do the right thing in the presence of nonlocal exits.

Regarding compatibility, we could #define it to nothing if we compile
without DISABLE_DEPRECATED.

Cheers,

Andy
-- 
http://wingolog.org/




^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: array handles and non-local exits
  2009-07-09 19:19                 ` Andy Wingo
@ 2009-07-09 21:08                   ` Ludovic Courtès
  2009-07-10 10:27                     ` Andy Wingo
  0 siblings, 1 reply; 23+ messages in thread
From: Ludovic Courtès @ 2009-07-09 21:08 UTC (permalink / raw)
  To: guile-devel

Hello,

Andy Wingo <wingo@pobox.com> writes:

> On Mon 06 Jul 2009 21:30, ludo@gnu.org (Ludovic Courtès) writes:

[...]

>> Yes.  OTOH, the doc doesn't say that concurrent array accesses are safe,
>> so array accesses are supposed to be synchronized at the application
>> level, using mutexes, I suppose.
>
> They should be safe in the sense that they shouldn't crash Guile, but
> the result may be strange -- e.g. hashtable insertion.

Yes, of course.

>> Still, I don't feel like we have any compelling reason to remove
>> `scm_array_handle_release ()'.  One argument to keep it is that it's the
>> kind of thing that's much easier to remove than to reinstate, and "we
>> never know".  Also, removing it would cause gratuitous
>> incompatibility.
>
> To me this is a weak argument, especially given that much code probably
> doesn't do the right thing in the presence of nonlocal exits.

To me, *this* is a weak argument.  ;-)

> Regarding compatibility, we could #define it to nothing if we compile
> without DISABLE_DEPRECATED.

Or we can always #define it to nothing.  From an API design viewpoint, I
find it consistent to have `release ()'.  If you're concerned about the
function call overhead, then turning it into a macro will address that
concern.  :-)

Thanks,
Ludo'.





^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: array handles and non-local exits
  2009-07-09 21:08                   ` Ludovic Courtès
@ 2009-07-10 10:27                     ` Andy Wingo
  2009-07-10 12:05                       ` Ludovic Courtès
  0 siblings, 1 reply; 23+ messages in thread
From: Andy Wingo @ 2009-07-10 10:27 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel

Hi,

On Thu 09 Jul 2009 22:08, ludo@gnu.org (Ludovic Courtès) writes:

>> To me this is a weak argument, especially given that much code probably
>> doesn't do the right thing in the presence of nonlocal exits.
>
> To me, *this* is a weak argument.  ;-)

Heh, allow me to elaborate then. "Let's keep this thing because maybe it
will be useful" is a form of cargo-culting. I am saying that it has no
use, and should be removed, *because* its presence has a cost and no
benefit. Not so much in time, but in code complexity. You can't just
write functions that return values, you have to contort code to store
temporaries and then release and then return. You have to write paired
statements. If you call a user function, you really should set up a
dynwind, which conses, and contorts your code even further.
Array_handle_release is a bad idea.

http://en.wikipedia.org/wiki/Cargo_cult_programming

Regards,

Andy
-- 
http://wingolog.org/




^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: array handles and non-local exits
  2009-07-10 10:27                     ` Andy Wingo
@ 2009-07-10 12:05                       ` Ludovic Courtès
  2009-07-19 20:04                         ` Neil Jerram
  0 siblings, 1 reply; 23+ messages in thread
From: Ludovic Courtès @ 2009-07-10 12:05 UTC (permalink / raw)
  To: guile-devel

Hey,

Andy Wingo <wingo@pobox.com> writes:

[...]

> You can't just write functions that return values, you have to contort
> code to store temporaries and then release and then return. You have
> to write paired statements. If you call a user function, you really
> should set up a dynwind, which conses, and contorts your code even
> further.

Right, that one has to set up a dynwind for nothing sucks.

> Array_handle_release is a bad idea.

Fair enough.

The doc would need to be revised (again).

(It would have helped in this discussion if we knew the rationale for
this API.  I couldn't find any trace of discussions around it.)

Thanks,
Ludo'.





^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: array handles and non-local exits
  2009-07-10 12:05                       ` Ludovic Courtès
@ 2009-07-19 20:04                         ` Neil Jerram
  2009-07-20  9:20                           ` Ludovic Courtès
  0 siblings, 1 reply; 23+ messages in thread
From: Neil Jerram @ 2009-07-19 20:04 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel

ludo@gnu.org (Ludovic Courtès) writes:

> Hey,
>
> Andy Wingo <wingo@pobox.com> writes:
>
> [...]
>
>> You can't just write functions that return values, you have to contort
>> code to store temporaries and then release and then return. You have
>> to write paired statements. If you call a user function, you really
>> should set up a dynwind, which conses, and contorts your code even
>> further.
>
> Right, that one has to set up a dynwind for nothing sucks.
>
>> Array_handle_release is a bad idea.
>
> Fair enough.

FWIW, I agree (I think with both of you) that `we might need it in
future' is not a good argument, but that API compatibility is.

> The doc would need to be revised (again).
>
> (It would have helped in this discussion if we knew the rationale for
> this API.  I couldn't find any trace of discussions around it.)

I'm pretty sure it was about allowing C code to efficiently access and
modify uniform vector contents, but at the same time supporting
operations which might require the underlying storage to be
reallocated.

The latter operations could include enlarging an existing vector, or
copy-on-write.  But AFAICT we never implemented either of those ideas,
and the existing code never changes the underlying storage of a
vector.

Regards,
        Neil




^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: array handles and non-local exits
  2009-07-19 20:04                         ` Neil Jerram
@ 2009-07-20  9:20                           ` Ludovic Courtès
  2009-07-23 21:20                             ` Andy Wingo
  0 siblings, 1 reply; 23+ messages in thread
From: Ludovic Courtès @ 2009-07-20  9:20 UTC (permalink / raw)
  To: guile-devel

Hi Neil,

Neil Jerram <neil@ossau.uklinux.net> writes:

> ludo@gnu.org (Ludovic Courtès) writes:
>
>> Andy Wingo <wingo@pobox.com> writes:

[...]

>>> Array_handle_release is a bad idea.
>>
>> Fair enough.
>
> FWIW, I agree (I think with both of you) that `we might need it in
> future' is not a good argument, but that API compatibility is.

OK.  So we can proceed with the removal, leaving a no-op macro when
SCM_ENABLE_DEPRECATED == 1.  Andy, can you take care of this?  :-)

> I'm pretty sure it was about allowing C code to efficiently access and
> modify uniform vector contents, but at the same time supporting
> operations which might require the underlying storage to be
> reallocated.
>
> The latter operations could include enlarging an existing vector, or
> copy-on-write.  But AFAICT we never implemented either of those ideas,
> and the existing code never changes the underlying storage of a
> vector.

Thanks for the information.

Ludo'.





^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: array handles and non-local exits
  2009-07-20  9:20                           ` Ludovic Courtès
@ 2009-07-23 21:20                             ` Andy Wingo
  0 siblings, 0 replies; 23+ messages in thread
From: Andy Wingo @ 2009-07-23 21:20 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel

Hi,

On Mon 20 Jul 2009 11:20, ludo@gnu.org (Ludovic Courtès) writes:

> Neil Jerram <neil@ossau.uklinux.net> writes:
>
>> ludo@gnu.org (Ludovic Courtès) writes:
>>
>>> Andy Wingo <wingo@pobox.com> writes:
>
> [...]
>
>>>> Array_handle_release is a bad idea.
>>>
>>> Fair enough.
>>
>> FWIW, I agree (I think with both of you) that `we might need it in
>> future' is not a good argument, but that API compatibility is.
>
> OK.  So we can proceed with the removal, leaving a no-op macro when
> SCM_ENABLE_DEPRECATED == 1.  Andy, can you take care of this?  :-)

Sure. I'd like to include it as part of wip-array-refactor, so I'll wait
and see what the resolution is there.

Andy
-- 
http://wingolog.org/




^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2009-07-23 21:20 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-09-15 20:17 array handles and non-local exits Neil Jerram
2008-09-16  7:56 ` Ludovic Courtès
2008-09-17 19:32   ` Neil Jerram
2008-09-18  8:15     ` Ludovic Courtès
2008-09-18  9:17       ` Neil Jerram
2008-09-18 13:44         ` Ludovic Courtès
2009-06-30 22:59           ` Neil Jerram
2009-07-01  8:37             ` Ludovic Courtès
2009-07-01 22:04               ` Neil Jerram
2009-07-01 22:36                 ` Ludovic Courtès
2009-07-02 23:33                   ` Neil Jerram
2009-07-03 23:38                     ` Neil Jerram
2009-07-04 19:28         ` Andy Wingo
2009-07-05 11:14           ` Ludovic Courtès
2009-07-06 14:09             ` Andy Wingo
2009-07-06 20:30               ` Ludovic Courtès
2009-07-09 19:19                 ` Andy Wingo
2009-07-09 21:08                   ` Ludovic Courtès
2009-07-10 10:27                     ` Andy Wingo
2009-07-10 12:05                       ` Ludovic Courtès
2009-07-19 20:04                         ` Neil Jerram
2009-07-20  9:20                           ` Ludovic Courtès
2009-07-23 21:20                             ` Andy Wingo

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).