Olaf Dabrunz wrote:

> > > Example:
> > > 
> > >     :let d = {'a': 99, 'b': 100}
> > >     :lockvar 1 d
> > >     :call extend(d, {'a': 123})
> > >         E741: Value is locked: extend() argument
> > >     :let d.a = 123
> > >     :let d
> > >         d                     {'a': 123, 'b': 100}
> > > 
> > > The dictionary is locked, but the 'a' item is not, so extend() on 'a'
> > > should succeed in the same way as the 'let' does. According to ':he
> > > lockvar', a dictionary lock protects against item deletion and addition,
> > > but not against changing an existing item.
> > 
> > Hmm, one could argue that extend() adds the new key/value pair, thus
> > that it should fail.
> 
> For lists, I agree -- extend(list, list2[, where]) does not touch
> existing items.
> 
> For dicts, extend() modifies existing items and adds new items when
> their keys do not exist yet.
> 
> > Zyx's argument that it may change one existing key/value but then fail
> > on a following one that adds a key makes sense.
> 
> This was also discussed by you and ZyX for patch 7.4.419 ("when part of
> a list is locked it's possible to make changes"), which is a related
> situation.  ZyX proposed several approaches, and the result was "[list]
> items will be removed [or changed] only if all items are not locked".
> 
> >                                                  Perhaps we should just
> > state "extend() on a locked list or dict does not work". 
> 
> If we let extend() fail here, then ':let' (and map()) should fail as
> well.  The lock should not have a different effect when the change is
> attempted by different means.

Well, it depends on how you look at it.  Although extend() can be used
to change the value of an existing item, it is not what it is normally
used for.  Using an assignment to an item with a specific key is a more
obvious way to change the value.  Of course, if the key does not exist
the assignment would fail if the dict is locked.

Also: It's really easy to document that extend() does not work on a
locked dictionary.  That is also an indication that this is easy to
understand.  Trying to explain that it might work in specific situations
is more complicated, indicating it's also harder to understand for
users.

> This would be a different lock model.
> 
> 
> The all-or-nothing semantics you and ZyX mention can be implemented for
> extend(dict, dict2[, how]) as well.  But note:
> 
>     - extend(dict, dict2[, how]) currently already has several
>       exit-on-error conditions:
> 
>         - adding a function to scopes l: or g: that conflicts with a
>           builtin function
> 
>         - adding or replacing an item in a variable scope, when the key
>           is not a valid variable name
> 
>         - changing an existing dict item while 'how' is 'error'
> 
>         - changing a locked dict item (since 7.4.698).
> 
>       For all-or-nothing semantics, all of these need to be moved into a
>       separate checking loop.
> 
>       (This is the reason why I considered all-or-nothing semantics
>       beyond the scope of my patch: the current implementation has
>       exit-on-error semantics.)
> 
> 
>     - A separate checking loop needs to repeat all dictionary lookups.
>       For my tests, this made extend(dict, ...) take twice as much time
>       to complete.

Yes, this would be the reason to only use this for error conditions that
do not require looping over all the items (in the dict and in the
arguments).

>       :let d = {}
>       :for i in range(10000) | let d[i] = 't'.i | endfor
>       :for t in range(3)
>       : let start = reltime()
>       : for i in range(10000) | call extend(d, d) | endfor
>       : echomsg reltimestr(reltime(start))
>       :endfor
> 
>       Unpatched:
> 
>           3.255444
>           3.243517
>           3.248523
> 
>       Old patch (exit-on-error semantics):
> 
>           3.249932
>           3.256253
>           3.255438
> 
>       New patch with separate checking loop (see below):
> 
>           6.452288
>           6.390474
>           6.420924
> 
>       I am not sure if taking twice as long is a good tradeoff for
>       having all-or-nothing semantics.

For things where the script writer made an obvious mistake, such as
trying to overwrite a builtin function, the slowness is not good.
For the cases where the dict would be halfway a change, it was already
modified when an unexpected error condition is encountered, it would be
worth it.

It's a tough decision.  In those cases I prefer to leave it as it is.

> 
>     - In case of an out of memory error, extend() skips that item and
>       continues with the next one (continue-after-error semantics).
> 
>       For OOM, all-or-nothing semantics can only be achieved by
>       implementing a rollback, because OOM can only happen when we
>       actually execute the changes.
> 
>       A rollback requires copies of the changed items.  Making them is
>       relatively expensive.
> 
>       Note that list functions do not implement this: 'extend(list,
>       list2, how)' and ':let list[i:j] = list2' and probably others do
>       have exit-on-OOM semantics.
> 
>       One reason for not implementing all-or-nothing for OOM may be that
>       OOM rarely happens: as discussed in another thread, users tend to
>       terminate other processes before OOM (to avoid a slow system that
>       is swapping or thrashing), or an OOM-killer may kill some other
>       process to avoid OOM.
> 
>       It may not be worth the slowdown just to have a rollback on OOM.
> 
> An updated patch that implements all-or-nothing semantics in
> extend(dict, dict2[, how]) (except for OOM) is at the end of this mail.
> 
> 
> On another note, while looking at the 7.4.419 patch again, I realized
> that it adds checks to ':unlet' for item locks before it deletes any
> items.  This is not needed under the locking model described in ':he
> lockvar': item locks protect against item change, not against item
> deletion.
> 
> The list lock, which protects against deletion, is checked before
> already.
> 
> Here is a patch that removes the unneeded item lock checks.  Note that
> no changes to the tests are needed, as the list lock already makes sure
> the items cannot be deleted.

So this makes extend() much slower.  Not sure what part of this I would
include.


-- 
Me?  A skeptic?  I trust you have proof.

 /// Bram Moolenaar -- [email protected] -- http://www.Moolenaar.net   \\\
///        sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\  an exciting new programming language -- http://www.Zimbu.org        ///
 \\\            help me help AIDS victims -- http://ICCF-Holland.org    ///

-- 
-- 
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php

--- 
You received this message because you are subscribed to the Google Groups 
"vim_dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Raspunde prin e-mail lui