On Sat, Mar 24, 2018 at 11:30 AM Matteo Merli <matteo.me...@gmail.com> 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 <guosi...@gmail.com> wrote: > > > On Sat, Mar 24, 2018 at 8:38 AM, Matteo Merli <mme...@apache.org> 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 > > > <mme...@apache.org> > > > > > > -- > Matteo Merli > <mme...@apache.org> >