Hi Qiang, Lastly, it would be beneficial to conduct a test on the distributions as the ConcurrentHashMap has demonstrated improved efficiency. However, a drawback is the potential increase in JVM GC overhead. If we can assess the final outcome of the Pulsar benchmark, the findings would be more persuasive.
Moreover, this process should not consume excessive time. Simply initiate the script, await the deployment, and execute the test. I have personally performed this procedure numerous times with success. :) Regards, Penghui On Wed, Sep 6, 2023 at 10:26 PM mattison chao <mattisonc...@gmail.com> wrote: > Hi, Guys. > > IMO, It's a data structure difference. We can give the benchmark of two > structures in different situations (pulsar using situations). > > we can built a distribution for the benchmark. After we get the result, > > then we can decide if it's fine to go to the new solution. The change > should also be > > applied to bookkeeper. And I can help to run the benchmark comparison by > using > > openmessaging benchmark and share the result here. > Hi, Penghui > > I am not sure if deserve to spend many time on distribution testing. but I > think other contributors would love it If we can get some comparison of two > different data structure. > > Best, > Mattison > On 6 Sep 2023 at 22:00 +0800, PengHui Li <peng...@apache.org>, wrote: > > I also support the idea. > > > > For the execution. I think we can have a branch to apply the changes and > > then > > we can built a distribution for the benchmark. After we get the result, > > then we can > > decide if it's fine to go to the new solution. The change should also be > > applied to > > bookkeeper. And I can help to run the benchmark comparison by using > > openmessaging > > benchmark and share the result here. > > > > Regards, > > Penghui > > > > On Wed, Sep 6, 2023 at 4:56 PM Yunze Xu <x...@apache.org> wrote: > > > > > I support replacing it with JDK's ConcurrentHashMap. Maintaining a > > > customized concurrent hash map whose algorithm is essentially the same > > > with the implementation of a very old version of JDK is painful. The > > > PROs listed like no boxing and linear probing have not proved to be > > > better by any benchmark. Instead, Penghui's benchmark shows the > > > performance is much worse. > > > > > > Regarding the remove or other operations in forEach method, I believe > > > there is a good way to resolve it and we should not do many things in > > > the forEach callback. > > > > > > Thanks, > > > Yunze > > > > > > On Wed, Sep 6, 2023 at 2:36 PM the tumbled <wof...@qq.com.invalid> > wrote: > > > > > > > > > > > > > > > > > > > > On 2023/09/06 04:22:58 QQ wrote: > > > > > > > Hi Pulsar Community, > > > > > > > > > > > > > > I’d like to start a discussion about whether replacing the > customize > > > util class like ConcurrentOpenHashMap with ConcurrentHashMap, as the > > > performance of ConcurrentHashMap is better than those customize util > > > significantly. > > > > > > > Worse, these customize util class cannot ensure consistence in > method > > > forEach as PR https://github.com/apache/pulsar/pull/21110 shows, > which is > > > disquieting, although it may not cause any problem. > > > > > > > > > > > > > > Thanks, > > > > > > > The Tumbled. > > > > > > > > > > > > > > > > > The benchmark results is provided in > > > https://github.com/apache/pulsar/pull/20647\#issuecomment-1607257960 < > > > https://github.com/apache/pulsar/pull/20647%5C#issuecomment-1607257960 > >. > > > > > > > > > > > > > > > > > However, iterators are designed to be used by only one thread > at a > > > time. Bear in mind that the results of aggregate status methods > including > > > size, isEmpty, and containsValue are typically useful only when a map > is > > > not undergoing concurrent updates in other threads. Otherwise the > results > > > of these methods reflect transient states that may be adequate for > > > monitoring or estimation purposes, but not for program control. > > > > > > > > > > And I notice that ConcurrentHashMap’s forEach method do not support > > > thread-safety too. > > > > > It seems that the only reason left for us to replacing the > customized > > > util is the superior performance of ConcurrentHashMap. > > > > > > > > > > Thanks, > > > > > The Tumbled. > > > >
