Merge lp://staging/~gary/launchpad/profiler into lp://staging/launchpad

I'd never played with this before, and now I'm very glad I have.

[1]

+ self.__class__.profiler_lock.acquire(True) # Blocks.

Is there a reason you chose to refer to profiler_lock via
self.__class__ rather than self?

[2]

+ actions = getattr(_profilers, 'actions', None)
+ profiler = getattr(_profilers, 'profiler', None)

Btw, instead of lots of guard code around using _profilers, it's
possible to subclass and get a ordinary looking object:

    class Profilers(threading.local):

        def __init__(self):
            self.profiling = False
            self.actions = None
            self.profiler = None
            self.profiling = False
            # ...

    _profilers = Profilers()

__init__() gets called once for each new thread the instance is used
in, and the class can have methods and properties as usual. Might be
interesting.

Fwiw, I verified this to myself with http://paste.ubuntu.com/663456/

I made all changes Gavin suggested.

QA: simply make sure that https://qastaging.launchpad.net/++profile++show still seems to work, as a smoke test.

Uhm so the reason I used the bzr profiler was *precisely* the threading behaviour - we have stuff that runs off in threads (like the google search calls), and relatedly I wanted to lock other threads out to ensure we got a good read - only one request active - when profiling.

Could we not just fix bzr to do what you need, given the closeness of the projects?

On Aug 11, 2011, at 11:02 PM, Robert Collins wrote:

> Review: Needs Information
> Uhm so the reason I used the bzr profiler was *precisely* the threading behaviour - we have stuff that runs off in threads (like the google search calls), and relatedly I wanted to lock other threads out to ensure we got a good read - only one request active - when profiling.

This is maintained: I kept the blocking behavior.

> Could we not just fix bzr to do what you need, given the closeness of the projects?

It would be possible. I don't see an advantage at all until we are happy with the profiler work; and even then, upstream inclusion, no matter how friendly, takes time and involves a different perspective.

If for some reason you want to use the exact lock from the bzr profiler, that would be easy enough to maintain, as I did in a previous branch. However, it is simpler to understand the code if we define the lock locally, as I did here.

On Wed, Aug 17, 2011 at 12:19 AM, Gary Poster <email address hidden> wrote:
>
> On Aug 11, 2011, at 11:02 PM, Robert Collins wrote:
>
>> Review: Needs Information
>> Uhm so the reason I used the bzr profiler was *precisely* the threading behaviour - we have stuff that runs off in threads (like the google search calls), and relatedly I wanted to lock other threads out to ensure we got a good read - only one request active - when profiling.
>
> This is maintained: I kept the blocking behavior.

Cool, thanks, and thanks for explaining.

>> Could we not just fix bzr to do what you need, given the closeness of the projects?
>
> It would be possible.  I don't see an advantage at all until we are happy with the profiler work; and even then, upstream inclusion, no matter how friendly, takes time and involves a different perspective.
>
> If for some reason you want to use the exact lock from the bzr profiler, that would be easy enough to maintain, as I did in a previous branch.  However, it is simpler to understand the code if we define the lock locally, as I did here.

Well, its a little sad to have two different implementations floating
around, is all.

-Rob

On Aug 16, 2011, at 8:31 AM, Robert Collins wrote:

> On Wed, Aug 17, 2011 at 12:19 AM, Gary Poster <email address hidden> wrote:
>>
>> On Aug 11, 2011, at 11:02 PM, Robert Collins wrote:

...

>>> Could we not just fix bzr to do what you need, given the closeness of the projects?
>>
>> It would be possible. I don't see an advantage at all until we are happy with the profiler work; and even then, upstream inclusion, no matter how friendly, takes time and involves a different perspective.
>>
>> If for some reason you want to use the exact lock from the bzr profiler, that would be easy enough to maintain, as I did in a previous branch. However, it is simpler to understand the code if we define the lock locally, as I did here.
>
> Well, its a little sad to have two different implementations floating
> around, is all.

True. It would be fun to contribute to bzr. Maybe I'll get around to it. I'd want to try to change things around a bit if it were going upstream: reuse often leads me to different designs than refactoring.