Proposed javac argument processing performance improvement

[Jonathan Gibbons]

On 06/16/2011 08:16 PM, David Schlosnagle wrote:
> On Tue, Jun 14, 2011 at 7:34 PM, Jonathan Gibbons
> <jonathan.gibbons at oracle.com>  wrote:
>> David,
>>
>> Looking at your patch, you take a hit at the end by converting the Set to a
>> List by way of an array.  At a minimum, it would be worth overloading
>> List.from to accept an Iterable<A>  -- that would allow you to eliminate the
>> intermediate array. For bonus points, you could change the return type of
>> processArgs to Iterable<File>  and avoid the copy altogether. However, in
>> that case, you would need to fix up a couple of isEmpty() calls in Main. The
>> primary use of the result is in Main.java:412, which already accepts
>> Iterable<File>.  There are no other uses that I can see within the main
>> javac source code. I found 3 uses in the regression tests that would be
>> unaffected.
> Jon,
>
> Thanks for the comments. I had debated refactoring the method
> signature, but held off initially. This weekend I'll try to put
> together a patch with Iterable<File>  and gather some more realistic
> stats across a few platforms. I did a few initial tests on a medium
> sized build and the results were very promising -- the total javac
> execution time for compiling a 5,522 file module went from 54 seconds
> to 39 seconds, a reduction of 15 seconds all due to reducing
> processArgs execution time from 15.890 seconds to 0.350 seconds. For
> smaller numbers of input files, the gains were proportional to the
> number of input files. With the Iterable changes, I hope to see even
> more gains.
>
>> I believe there are a couple of regression tests that need to be fixed up if
>> you change the type of JavaCompiler.filenames.
> I'll include these changes in the patch and run jtreg. Do you
> typically create performance related regression tests? Are there any
> new tests you'd recommend for me to add?
>
> On a side note, are there any langtools jtreg tests that are currently
> known to fail? I saw 3 failed javadoc tests that seem to be Mac OS X
> specific and probably related to
> http://java.net/jira/browse/MACOSX_PORT-38.
>
> - Dave

Dave,

Thanks for the update, and for looking at making your patch work even 
faster.

For a perf-related change like this, no new regression tests will be 
necessary.
The normal rules in a case like this are to make sure that nothing is 
broken,
meaning that all the (langtools) tests should still pass and that it 
should still
be capable to build the OpenJDK forest. Normally, it is sufficient to do 
a regular
build of OpenJDK; in some circumstances, it would be expected to pass a
build with SKIP_BOOT_CYCLE=false, meaning that it should be able to build
OpenJDK -- and the result should itself be capable of building OpenJDK. This
double build verifies that the bits from the first build are good enough to
function as a BOOT_JDK.  But, I don't think the double build is 
necessary in this case,
since this is a relatively simple change related to command line processing.

Normally, all the langtools tests should pass all time time, on all 
supported
platforms (Linux, Windows, Solaris.) I haven't checked out the Mac port 
recently
to see how the tests perform on that platform.

-- Jon