[Piglit] [Patch v2 1/3] framework.log: Replace synchronized_self decorator

[Ilia Mirkin]

On Fri, Apr 25, 2014 at 4:45 PM, Dylan Baker <baker.dylan.c at gmail.com> wrote:
> On Friday, April 25, 2014 16:40:36 Ilia Mirkin wrote:
>
>> On Fri, Apr 25, 2014 at 4:38 PM, Dylan Baker <baker.dylan.c at gmail.com>
>> wrote:
>
>> > This replaces the synchronized_self decorator from piglit's threads
>
>> > module with a context manager (with foo: <stuff>). This is more standard
>
>> > python, and means having a little less hand rolled code around.
>
>> >
>
>> > v2: - fix mixed usage of __lock and _lock
>
>>
>
>> There's probably an obvious reason for this, but I'm missing it -- why
>
>> are you using _lock when all the other vars are hidden with __?
>
>
>
> I've been bitten too many times now using __, I'm trying not to use that
> mechanism unless I can come up with a good reason to.
>
>
>
> Unless I'm completely misunderstanding python style, a leading _ is the
> equivalent what ruby calls a protected member: That class and all of it's
> children have access to that member. In python it's only suggested that
> method not be called outside of the class. and leading __ is the equivalent
> of a 'private' method, only that class, and not it's children, can access
> that member.
>

I think that's roughly correct -- if you have a member named __foo, it
gets auto-renamed to some random string (so, still accessible for the
very determined, just not under its original name -- I think it's
hashed with the defining class's name or something along those lines).
Having _foo being protected is merely a convention -- nothing enforces
it.

>
>
> Which means I did the wrong thing when I wrote Log. Unless I completely
> misunderstood?

Not sure why __ is the wrong thing for log's private members. Or why
you wouldn't want lock to be considered private.

  -ilia

>
>
>
>>
>
>> > Signed-off-by: Dylan Baker <baker.dylan.c at gmail.com>
>
>> > ---
>
>> >
>
>> > framework/log.py | 31 ++++++++++++++++---------------
>
>> > 1 file changed, 16 insertions(+), 15 deletions(-)
>
>> >
>
>> > diff --git a/framework/log.py b/framework/log.py
>
>> > index d045847..77cc02a 100644
>
>> > --- a/framework/log.py
>
>> > +++ b/framework/log.py
>
>> > @@ -22,7 +22,7 @@
>
>> >
>
>> > import sys
>
>> > import collections
>
>> >
>
>> > -from .threads import synchronized_self
>
>> > +from threading import RLock
>
>> >
>
>> > class Log(object):
>
>> > @@ -38,6 +38,7 @@ class Log(object):
>
>> > self.__running = []
>
>> > self.__generator = (x for x in xrange(self.__total))
>
>> > self.__pad = len(str(self.__total))
>
>> >
>
>> > + self._lock = RLock()
>
>> >
>
>> > self.__summary_keys = set(['pass', 'fail', 'warn', 'crash',
>
>> > 'skip',
>
>> >
>
>> > 'dmesg-warn', 'dmesg-fail',
>
>> > 'dry-run'])
>
>> >
>
>> > self.__summary = collections.defaultdict(lambda: 0)
>
>> >
>
>> > @@ -89,7 +90,6 @@ class Log(object):
>
>> > 'name': name,
>
>> > 'result': result}))
>
>> >
>
>> > - @synchronized_self
>
>> >
>
>> > def post_log(self, value, result):
>
>> > """ Used to mark a test as complete in the log
>
>> >
>
>> > @@ -98,17 +98,17 @@ class Log(object):
>
>> > result -- the result of the completed test
>
>> >
>
>> > """
>
>> >
>
>> > - # Mark running complete
>
>> > - assert value in self.__running
>
>> > - self.__running.remove(value)
>
>> > + with self._lock:
>
>> > + # Mark running complete
>
>> > + assert value in self.__running
>
>> > + self.__running.remove(value)
>
>> >
>
>> > - # increment the number of completed tests
>
>> > - self.__complete += 1
>
>> > + # increment the number of completed tests
>
>> > + self.__complete += 1
>
>> >
>
>> > - assert result in self.__summary_keys
>
>> > - self.__summary[result] += 1
>
>> > + assert result in self.__summary_keys
>
>> > + self.__summary[result] += 1
>
>> >
>
>> > - @synchronized_self
>
>> >
>
>> > def log(self, name, result):
>
>> > """ Print to the screen
>
>> >
>
>> > @@ -116,10 +116,10 @@ class Log(object):
>
>> > over it.
>
>> >
>
>> > """
>
>> >
>
>> > - assert result in self.__summary_keys
>
>> > - self.__print(name, result)
>
>> > + with self._lock:
>
>> > + assert result in self.__summary_keys
>
>> > + self.__print(name, result)
>
>> >
>
>> > - @synchronized_self
>
>> >
>
>> > def pre_log(self, running=None):
>
>> > """ Hook to run before log()
>
>> >
>
>> > @@ -131,8 +131,9 @@ class Log(object):
>
>> > nothing will be printed. Default: None
>
>> >
>
>> > """
>
>> >
>
>> > - if running:
>
>> > - self.__print(running, 'running')
>
>> > + with self._lock:
>
>> > + if running:
>
>> > + self.__print(running, 'running')
>
>> >
>
>> > x = self.__generator.next()
>
>> > self.__running.append(x)
>
>> >
>
>> > --
>
>> > 2.0.0.rc0
>
>> >
>
>> > _______________________________________________
>
>> > Piglit mailing list
>
>> > Piglit at lists.freedesktop.org
>
>> > http://lists.freedesktop.org/mailman/listinfo/piglit
>
>