[
Date Prev][
Date Next][
Thread Prev][
Thread Next][
Date Index][
Thread Index]
[
List Home]
|
Re: [jdt-dev] Performance degradation switching from 1.8 to 11
|
Hi Gunnar,
Thanks for the effort! Personally, I would go for option 3 - completely switch to collections API. However, as Andrey pointed out, this may have a few memory/other issues. Hence, my recommendation would be to do this effort spread across a few milestones, for eg, you can incorporate one part of the changes for M1 to start with [after appropriate perf/mem checks].
Opinions, anyone else?
Regards,
Manoj
-----jdt-dev-bounces@xxxxxxxxxxx wrote: -----
To: "Eclipse JDT general developers list." <jdt-dev@xxxxxxxxxxx>
From: Gunnar Wagenknecht
Sent by: jdt-dev-bounces@xxxxxxxxxxx
Date: 09/30/2020 12:16AM
Subject: [EXTERNAL] Re: [jdt-dev] Performance degradation switching from 1.8 to 11
I have a general question while I was working on this.
I did a couple of changes by replacing the implementation of those classes. I discovered some "convenience" methods which are different that the contacts in standard collections (eg., #put returning the *new* value instead of the existing). I kept those to not cause too much changes for now.
What is the JDT team's preference here? We have a couple options.
(1) Hide HashMap inside existing implementations
Example:
https://git.eclipse.org/r/c/jdt/eclipse.jdt.core/+/169915/2/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/util/HashtableOfPackage.java
- callers need to be changed if they access underlying arrays directly
- additional indirection is added
(2) Make existing implementation extend HashMap
Example:
https://git.eclipse.org/r/c/jdt/eclipse.jdt.core/+/170026/1/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/util/HashtableOfType.java
- callers need to be changed if they access underlying arrays directly
+ class becomes a drop-in replacement for collections with convenient methods
- callers have more flexibility (eg., stream api) but need to pay attention to details (eg., arrays keys wrapped into key object)
(3) Remove the existing implementation entirely
- we eliminate the legacy classes
- all callers need to be switched to collections api
- still need to introduce key object for arrays (for the desired hashCode/equals implementation)
My internal testing confirmed that they improve the situation at scale. Also, it looks like those modifications to not regress JDT performance for the default users. There is a cost of memory, which I haven't measured tbh and don't really think it's an important goal these days (compared to performance).
However, to be fair, the pure performance boost in my case does not come from those modifications. It's a "simple" loop optimization from O(N IPackageFragmentRoot.total.on.classpath) to O(1 map-lookup + result.size).
https://git.eclipse.org/r/c/jdt/eclipse.jdt.core/+/170035/1
Should I continue with those efforts of changing the data structures or is this not something the JDT team is looking at adopting?
Thoughts?
-Gunnar
--
Gunnar Wagenknecht
gunnar@xxxxxxxxxxxxxxx, http://guw.io/
> On Sep 24, 2020, at 20:22, Andrey Loskutov <loskutov@xxxxxx> wrote:
>
> All of the org.eclipse.jdt.internal.compiler.util classes are *memory* optimized, they were all written before Java 1.2 / modern non-synchronized Collections API appeared and are basically a various copies of SimpleLookupTable. Also HashtableOfArrayToObject that is responsible for the performance of org.eclipse.jdt.internal.core.NameLookup.isPackage(String[]) is another clone of this table. You can find overall in JDT clones of the original code - some with later fixes, some not.
>
> In all our latest performance related bugs in this area we saw that the replacing use of these ancient jdt *Table structures with default HashMap/HahSet increases throughput but costs some more memory. Especially worrying is the worst case behavior of those tables/sets, they do not scale from some 10.000+ of entries anymore, and they rehash() function that is invoked if the arrays go out of free space is extremely inefficient and may block for minutes (!) on big input on fastest workstations.
>
> So there is a big room for an improvement, but also a big chance to break JDT (I believe we did it every time we touched this code, there are very subtle implementation details that you only understand after you realized that you broke the build).
>
> See https://bugs.eclipse.org/bugs/show_bug.cgi?id=563030#c9 for one example (10% memory trade off for > 1000 % speedup in some worst cases for huge tables).
>
> See https://bugs.eclipse.org/bugs/show_bug.cgi?id=529984 discussion for some profiler results.
>
> See https://bugs.eclipse.org/bugs/show_bug.cgi?id=564092 as one of many places where the current code could be changed.
>
> Kind regards,
> Andrey Loskutov
>
> Спасение утопающих - дело рук самих утопающих
>
> https://www.eclipse.org/user/aloskutov
>
>
>> Gesendet: Donnerstag, 24. September 2020 um 18:35 Uhr
>> Von: "Gunnar Wagenknecht" <gunnar@xxxxxxxxxxxxxxx>
>> An: "Eclipse JDT general developers list." <jdt-dev@xxxxxxxxxxx>
>> Betreff: Re: [jdt-dev] Performance degradation switching from 1.8 to 11
>>
>> Hi All,
>>
>> I'm still investigating this one. I've attached two profiling samples of the same Code Assist operation to the bug. Based on this I have a couple of questions/ideas and looking for feedback whether it makes sense to continue/experimenting with something or not.
>>
>> Obviously the challenge here is creating a public, reproducible data set. Hence my call for ideas of things I can/should try in the code base. Any ideas are welcome.
>>
>>
>> LookupEnvironment.java:1620 org.eclipse.jdt.internal.compiler.util.HashtableOfPackage.get(char[])
>> --> takes 1 second
>>
>> Does this smell like potential hash collisions?
>>
>> There are more similar classes in the util package. Are those performance optimized? Would it make sense to start looking into replacing some of these with HashMaps?
>> I was thinking this because of https://openjdk.java.net/jeps/180 , which looks promising.
>>
>> I'm looking for someone with triable knowledge in this area.
>>
>>
>> SearchableEnvironment.java:922 org.eclipse.jdt.internal.core.NameLookup.isPackage(String[], IPackageFragmentRoot[])
>> --> takes 1 second in String.join
>>
>> This is quite surprising to me. It shows up two times in a single sample and contributing two seconds here to the execution time.
>>
>>
>> Any thoughts?
>>
>> -Gunnar
>>
>> --
>> Gunnar Wagenknecht
>> gunnar@xxxxxxxxxxxxxxx, http://guw.io/
>>
>>
>>> On Aug 29, 2020, at 11:03, Gunnar Wagenknecht <gunnar@xxxxxxxxxxxxxxx> wrote:
>>>
>>> Greetings JDT Developers,
>>>
>>> We are seeing a severe performance degradation with Eclipse switching a large code base from 1.8 to 11. It's noticeable across our entire Eclipse use base. Increasing Eclipse JVM heap to 32GB on desktops with >= 64GB memory does not improve the situation.
>>>
>>> We are currently running Eclipse with OpenJDK 1.8.0_212 and the following additional options:
>>> -Xverify:none
>>> -XX:+UseStringDeduplication
>>> -XX:+DisableExplicitGC
>>> -XX:+ParallelRefProcEnabled
>>> -Dorg.eclipse.jdt.core.javamodelcache.ratio=1.5
>>> -Dorg.eclipse.jdt.core.javamodelcache.jartyperatio=4
>>>
>>> I've created https://bugs.eclipse.org/566498 for sharing logs and profiling results. I'd appreciate your input on thing to try and/or data you would like us to collect.
>>>
>>> Please let me know if you prefer communication on the bug of on the mailing list.
>>>
>>> Thanks a lot!
>>>
>>> -Gunnar
>>>
>>> --
>>> Gunnar Wagenknecht
>>> gunnar@xxxxxxxxxxxxxxx, http://guw.io/
>>>
>>>
>>
>> _______________________________________________
>> jdt-dev mailing list
>> jdt-dev@xxxxxxxxxxx
>> To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/jdt-dev
>>
> _______________________________________________
> jdt-dev mailing list
> jdt-dev@xxxxxxxxxxx
> To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/jdt-dev
_______________________________________________
jdt-dev mailing list
jdt-dev@xxxxxxxxxxx
To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/jdt-dev
