Skip to content
This repository was archived by the owner on Feb 13, 2025. It is now read-only.

Make an atomic context manager #13

Closed
ghost opened this issue Nov 29, 2013 · 7 comments
Closed

Make an atomic context manager #13

ghost opened this issue Nov 29, 2013 · 7 comments

Comments

@ghost
Copy link

ghost commented Nov 29, 2013

Originally reported by: RMTEW FULL NAME (Bitbucket: rmtew, GitHub: rmtew)


(originally reported in Trac by @ctismer on 2013-01-27 03:16:39)

Thinking of this example of atomic (taken as-is)

#!python
def acquire_lock(self):
  old = stackless.setatomic(1)
  if self.free:
    self.free = False:
  else:
    self.channel.receive()
  stackless.setatomic(old)

I felt it would make sense to make a context manager:

#!python
def acquire_lock(self):
  with stackless.atomic()
    if self.free:
      self.free = False:
    else:
      self.channel.receive()

See http://www.python.org/dev/peps/pep-0343/

What do you think:

  • Does it make sense to add that?
  • do we need any arguments?
  • makes sense as a builtin?

Does the syntax make sense, or is there a usecase that operates on anything else than
//stackless.getcurrent()//?


@ghost
Copy link
Author

ghost commented Nov 29, 2013

Original comment by RMTEW FULL NAME (Bitbucket: rmtew, GitHub: rmtew):


@krisvale on 2013-01-30 12:01:01 said:

There is one in stacklesslib.util

It´s like this:

@contextlib.contextmanager
def atomic():
    """a context manager to make the tasklet atomic for the duration"""
    c = stackless.getcurrent()
    old = c.set_atomic(True)
    try:
        yield
    finally:
        c.set_atomic(old)

The only thing that is annoying is that a local instance is required to hold the previous value.
It would be nicer if c.set_atomic would take an integer increment, so that we could do:

class atomic(object):
    """a context manager to make the tasklet atomic for the duration"""
    def __enter__(self):
        stackless.getcurrent().set_atomic(inc=1)
    def __exit__(self, e, v, t):
        stackless.getcurrent().set_atomic(inc=-1)
    old = c.set_atomic(inc=1)
atomic = atomic()

and then:

with atomic:
  foo()
  bar()

A stateless context manager has performance benefits.
I just haven't been able to come up with a clever interface. Should we add new functions (inc_atomic/dec_atomic) maybe?

@ghost
Copy link
Author

ghost commented Nov 29, 2013

Original comment by RMTEW FULL NAME (Bitbucket: rmtew, GitHub: rmtew):


@krisvale on 2013-01-30 12:54:40 said:

A context manager can be stateless.
My example, the one written using the class statement, maintains no state, it merely calls different functions before when entered and exit.
Notice how I create a singleton instance:

atomic = atomic() #replace the class with a singleton instance

# And here we use the singleton:
with atomic:
    foo()
    bar()

There are other exapmples of useful singleton context managers, such as context managers that translate exceptions but otherwise need not hold on to any data in self.

@ghost
Copy link
Author

ghost commented Nov 29, 2013

Original comment by RMTEW FULL NAME (Bitbucket: rmtew, GitHub: rmtew):


@ctismer on 2013-01-30 12:46:34 said:

The generator based implementation is very ok, IMHO.
A context manager is never stateless, because it is meant to restore a state
aber some action, which is written in one line, although it takes two actions,
so there always is state.

Of course it might be more efficient to avoid the creation of an object. Did you mean that?
But then you have to create a class-like thing, which again has similar cost.

Maybe this can be optimized quite much: I think the atomic feature always makes only
sense for the current tasklet, and there could be a singleton stackless.atomic object
that acts as a proxy to the current tasklet.
Is that what you meant in the example above?

#!python
with stackless.atomic:
    foo()
    bar()

and you even avoid the atomic() call?
I would propose a stackless.atomic object with enter and exit calls,
not exposing new functions outside, but just that.

@ghost
Copy link
Author

ghost commented Nov 29, 2013

Original comment by RMTEW FULL NAME (Bitbucket: rmtew, GitHub: rmtew):


@ctismer on 2013-01-30 13:20:27 said:

Yes, I understand. No state in self, but state elsewhere, in this case a counter in the tasklet.

But we are on the same track, using a singleton to use for the with, and this singleton needs to be created
only once and acts as a proxy to the current tasklet's atomic flag.
I was just not sure if more than a flag would be worth it. Would you spend a whole counter for every
tasklet just to save a rarely used tiny object creation?
Maybe it makes sense, I'm undecided there like I was in the initial stackless implementation.

A completely different approach:
There could be an atomic list of tasklets that holds all tasklets that are currently atomic.
This could even be extended to an atomic list of lists, where a tasklet can move from one list
of atomics to another one, dynamically allocated as needed. This would save the need to
maintain an extra counter, and on the average there would be just a fixed number of allocations.
A list of sets could also be used, whatever if more efficient.
It is the question if this would even allow to remove the atomic flag and use the set membership,
instead.
In any case, anything that does not need python code is probably more efficient than contextlib.
It is just the question if this makes sense enough to try an implementation at all?

@ghost
Copy link
Author

ghost commented Nov 29, 2013

Original comment by RMTEW FULL NAME (Bitbucket: rmtew, GitHub: rmtew):


@ctismer on 2013-01-30 16:16:38 said:

On list/dict/set:

I meant a list of lists/dict/set with the list index as the nesting level.
An atomic tasklet would be added to list[0]. A nested atomic would insert it also in list[1], and so on.
But maybe that's overkill.

On atomic state: The reason why I had that flag in the tasklet was my uncertaincy concerning a tasklet
switch inside the atomic section. Switching away from the atomic current to somewhere else would
leave the atomic context and re-enter it later. This could happen many times, so the atomic state
really belongs to the tasklet, if that feature was well-designed at all.

In addition to the local with context, it was also meant as a permanent setting, if a tasklet simply
should not be affected by preemption (stackless.run(n)). So far I also think it should be kept tasklet-wise.
But the with construct always means "now and here".
Btw., against the counter approach is the availability of the flag itself, and with means "''whatever is done to the
tasklet meanwhile, the old atomic state will be restored after the'' with".

@ghost
Copy link
Author

ghost commented Nov 29, 2013

Original comment by RMTEW FULL NAME (Bitbucket: rmtew, GitHub: rmtew):


@krisvale on 2013-01-30 14:38:14 said:

Yes, but I would consider this an implementation detail. Although having a stackless module level function rather than a tasklet function to affect this is appealing. One should only ever modify one's own 'atomic' state.

stackless.inc_atomic()
stackless.dec_atomic()

Also, implementation wise, if you decide to have a global list or dict, it still needs to cope with the concept of "level" in order to cope with nested usage of 'atomic'.
But I actually prefer the per-tasklet flag for its simplicity.

@ghost
Copy link
Author

ghost commented Jan 8, 2014

Original comment by Anonymous:


this has been done already in

  • revision 82885 branch 2.7-slp

  • revision 82905 branch 3.2-slp

at 2013-08-28 by Kristjan Valur Johnsson

@ghost ghost closed this as completed Sep 24, 2017
akruis pushed a commit that referenced this issue Nov 1, 2017
If the IRC notification is stored in plaintext, then anyone who forks
the repository and also adds it to travis will send notifications to
the IRC channel for their fork by default. Since the secure variable
is encrypted using a repository specific key, this will only work when
it is being built using the correct repository.
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

0 participants