OrderedScheduler and OrderedSafeExecutor
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. The main problem is that since OrderedScheduler supports both immediate and delayed execution, it uses a ScheduledThreadPoolExecutor per each bucket, plus some decorating wrappers. 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
Re: OrderedScheduler and OrderedSafeExecutor
Il sab 24 mar 2018, 16:38 Matteo Merli ha scritto: > 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. > Good catch IMHO if you can find a way to simply existing classes and preserve BK functionalities any change will be appreciated. I know that they are 'limited private' classes but only users of them are Pulsar and DL (which now has been merged to main codebase). This to say that you have a full vision about those classes. Enrico > The main problem is that since OrderedScheduler supports both immediate and > delayed execution, it uses a ScheduledThreadPoolExecutor per each bucket, > plus some decorating wrappers. > > 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 > > -- -- Enrico Olivelli
Re: OrderedScheduler and OrderedSafeExecutor
On Sat, Mar 24, 2018 at 8:38 AM, Matteo Merli 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 > >
Re: OrderedScheduler and OrderedSafeExecutor
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 wrote: > On Sat, Mar 24, 2018 at 8:38 AM, Matteo Merli 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 > > > > > -- Matteo Merli
Re: OrderedScheduler and OrderedSafeExecutor
On Sat, Mar 24, 2018 at 11:30 AM Matteo Merli 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 wrote: > > > On Sat, Mar 24, 2018 at 8:38 AM, Matteo Merli 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 > > > > > > > > > -- > Matteo Merli > >
