Community
Participate
Working Groups
I20061017-0800 I profiled a scenario from bug 139798 and found out that one of the top 5 time wasters is JDT Core by catching IOOBE instead of checking the condition upfront and accessing the array only once. Here are the steps: 1. start fresh workspace 2. import SWT plug-in 3. open class OS 4. Ctrl+O 5. type 'k' 6. type backspace Profile step 6 only. NOTES: 1) not sure whether all VMs perform better when checking the condition instead of catching the exception but all the ones I tried do so 2) other places in JDT Core seem to suffer from the same pattern
Created attachment 52214 [details] Simple test code showing that it takes longer catching an IOOBE
Created attachment 52216 [details] Fix for the reported use case The attached patch makes the reported use case about 400ms faster and is in my opinion also easier to understand. It does not other places in the code that have the same pattern. Places - like this one in the scanner - where the code is executed many times are candidates.
Created attachment 52217 [details] Profiler output
Interesting data. We will investigate. For scanner, the reasoning has always been that you hit the end of file only once per file, vs. a range check for every single char read...
Do you expect all IOOBE and AIOOBE catching to be replaced by bounds checks or simply the occurances in getNextToken() ?
It is faster to use an exception for source files, but it is faster to use bound checks when the size is small. In the case of the JavaConventions call, the size is always very small (check for identifiers, ...). So to improve this, we might want to have different reading methods in the scanner. I'll prepare a patch for all usage of this exception in the scanner and we can check with the performance tests where we are.
>It is faster to use an exception for source files,... Can you explain this? Doesn't my code example show that bounds testing is always faster than checking the AIOOB exception? > but it is faster to use >bound checks when the size is small. Mmh I tend to disagree (unless we use a different definition of "small": what made me file this bug in the first place was bug 139798 where a really large source (OS.java) with many members showed problems *and* with my patch the performance problem went away. Re comment 2: my comment "It does not other places in the code that have the same pattern." was probably not easy to parse ;-) ==> I wanted to say that I did not look for other instances of the same pattern in your code but there might be such. Re comment 5: when I talked to Philippe we agreed check those places where the impact is big i.e. inside (nested) loops or methods that are called very often.
(In reply to comment #7) > Can you explain this? Doesn't my code example show that bounds testing is > always faster than checking the AIOOB exception? When we parse a source file, we have one exception thrown. This is faster than checking the bounds for each character read. > Mmh I tend to disagree (unless we use a different definition of "small": what > made me file this bug in the first place was bug 139798 where a really large > source (OS.java) with many members showed problems *and* with my patch the > performance problem went away. In your case you are checking java identifier. These are small inputs. So in this case we should always perform a bound check. Your patch doesn't work as is. It won't scan unicodes properly. It needs to be reviewed. I am modifying all places where we catch exceptions, but this is not an easy task. We have many places. I'll run our performance tests once we are done.
Sorry about the problem with the patch.
What really needs to be optimized is the scanning of identifiers. This is used in the JavaConventions method and DOM SimpleName methods. This will need a good fix for bug 163349. The fixes for this bug and bug 163349 need to be released simultaneously.
Released for 3.3M4. Daniel, please verify your scenario with next integration build.
Verified in I20061107-0800.
*** Bug 168761 has been marked as a duplicate of this bug. ***
