git.net

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: OrderedScheduler and OrderedSafeExecutor


On Sat, Mar 24, 2018 at 11:30 AM Matteo Merli <matteo.merli@xxxxxxxxx>
wrote:

> One other thing that is not clear to me is why we need delayed execution
> AND key ordering.
>
> Shouldn't be fine to retry later in a random thread? If the requirement is
> to do something from a specific thread, we can also jump into that thread
> when the delay task is finally executed.


If I remember correctly, DL uses that for scheduling periodical flushes. We
need key ordering in that case. But I can be wrong since I am not close to
a laptop, can’t verify at this moment.

>
>
>
> On Sat, Mar 24, 2018 at 11:16 AM Sijie Guo <guosijie@xxxxxxxxx> wrote:
>
> > On Sat, Mar 24, 2018 at 8:38 AM, Matteo Merli <mmerli@xxxxxxxxxx> wrote:
> >
> > > While profiling the allocations of a BookKeeper client (from current
> > > master) writing entries, I've noticed that there are multiple
> allocations
> > > per entry related to the OrderedScheduler.
> > >
> > > I think OrderedScheduler was introduced in 4.6 and now
> > OrderedSafeExecutor
> > > is just an extension of OrderedScheduler as well.
> > >
> >
> > OrderedSafeExecutor extends OrderedScheduler was for BC. We made common
> > util classes in bookkeeper-common.
> >
> >
> > >
> > > The main problem is that since OrderedScheduler supports both immediate
> > and
> > > delayed execution, it uses a ScheduledThreadPoolExecutor per each
> bucket,
> > > plus some decorating wrappers.
> > >
> >
> > Sound like this is the solution:
> >
> > - Keep the OrderedScheduler
> > - Copy the OrderedScheduler to OrderedExecutor and change it to the
> > original executor implementation
> > - Let OrderedSafeScheduler extends OrderedExecutor (bc purpose)
> >
> >
> > >
> > > The ScheduledThreadPoolExecutor, needs a Priority queue and also always
> > > return future objects.
> > >
> > > From my memory snapshot I have counted 12 objects allocated (per
> addEntry
> > > operation) for a total of 472 bytes per each entry, all due the
> scheduled
> > > executor.
> > >
> > > I think the delayed execution is only used in unit tests. In critical
> > paths
> > > we're only using the immediate task submission.
> > >
> > > I think it might be worth to refactor OrderedScheduler or
> > > OrderedSafeExecutor to avoid that overhead.
> > >
> > > Thoughts?
> > >
> > > Matteo
> > > --
> > > Matteo Merli
> > > <mmerli@xxxxxxxxxx>
> > >
> >
> --
> Matteo Merli
> <mmerli@xxxxxxxxxx>
>