On Fri, 9 Oct 2020 05:31:54 GMT, yosbits <github.com+7517141+yos...@openjdk.org> wrote:
>> Before I review the actual proposed change, I have a pair of related >> high-level questions that I should have asked at >> the beginning of this review. >> 1. What is the expected performance gain, and under what conditions would >> you see the gain? >> 2. Do you have a program that demonstrates / measures the performance gain? >> >> It doesn't need to be something that would be suitable for including as part >> of the PR, but we will need some test >> program that shows enough of a performance gain to justify making this >> change. > > The performance improvements of the first change were self-evident, but > I agree that the current changes need to be explained. > > BitSet Features > * When using BitSet, memory usage (N/8) is wasted in the case of removing the > last one in the list. This is often caused > by user actions. > > PR Features > * The proposal records the run length of the flag, so the memory usage can be > kept small in the case of consecutive > deletion indexes. The worst case is the case where the deletion indexes are > not contiguous, which results in 4*N memory > consumption, but this happens very rarely. It's also the same as the memory > usage of getSelectedIndices(). (Memory > usage is further improved by setting it to int[]). > * Also, the delete loop is less CPU intensive and faster than BitSet's flag > scan. > > I will attach a test, but you'll have to wait until my next leisure time. I understand that this will improve the performance of this method in some cases (but not all as you correctly point out), but what I really meant by my questions was: When does this matter to an application's overall performance and what is the performance gain you would expect to see? As a general rule, we do not make this sort of performance optimization without a compelling reason. Improving the performance of a method is not by itself compelling unless there is a demonstrable (not theoretical) improvement to applications. I look forward to seeing the test program that shows the performance problem with the current implementation and allows us to measure the improvement with your proposed patch. ------------- PR: https://git.openjdk.java.net/jfx/pull/305
