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. 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>
