Proposed javac argument processing performance improvement

[Jonathan Gibbons]

David,

Thank you for your updated patch.  I agree with concern that the revised 
code
in JavacTaskImpl.java is somewhat messier than before, so I've attached an
alternate way of doing it for your consideration.  This keeps the code 
closer
to what it was before, at the expense of a couple of trivial utility 
methods.

If you agree with this alternate impl for JavacTaskImpl.java, we can proceed
by using that with the rest of your patch.

-- Jon

On 06/21/2011 05:40 AM, David Schlosnagle wrote:
> On Fri, Jun 17, 2011 at 12:03 AM, Jonathan Gibbons
> <jonathan.gibbons at oracle.com>  wrote:
>> 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
> Here's the updated patch using Iterable<File>  throughout, and the
> results are similar to the previous patch. Let me know if there is
> anything else I can do to get this through the process.
>
> On an aside, I tried to keep the IllegalArgumentException thrown in
> JavacTaskImpl consistent with the previous message; however, the code
> is much messier now  (this just begs for something like Guava's
> Joiner) and I'd prefer using something simpler along the lines of
> throw new IllegalArgumentException("Malformed arguments " +
> filenames);. That would have the realistic effect of moving from a
> space delimited list of filename arguments to a comma and space
> delimited and surrounded by square brackets per
> AbstractCollection#toString().
>
> I've included timings for several different size real world modules
> compiled with javac on Windows XP SP2 (where I had seen the worst
> performance to start). All times are in milliseconds. With the
> changes, processArgs is consistently sub-second, and linear in time
> relative to the number of input files.
>
>          |        Before       |         After       |          Delta        |
> # files | processArgs | javac | processArgs | javac | processArgs | javac   |
> --------+-------------+-------+-------------+-------+-------------+---------+
>     5522 |   16170.863 | 48163 |     280.484 | 37087 |  -15890.379 |  -11075 |
>     4436 |    9498.519 | 61043 |     200.735 | 49452 |   -9297.784 |  -11591 |
>      268 |      57.270 |  5775 |      16.923 |  5641 |     -40.347 |    -134 |
>      154 |      28.571 |  3147 |      11.322 |  2985 |     -17.249 |    -161 |
>      101 |      14.486 | 40621 |       7.814 | 40214 |      -6.672 |    -407 |
>       73 |      12.277 | 12180 |       6.677 | 12125 |      -5.600 |     -55 |
>        8 |       1.695 |  1718 |       1.309 |  1643 |      -0.386 |     -74 |
>        4 |       1.500 |  2098 |       1.399 |  2000 |      -0.101 |     -98 |
>        1 |       1.035 |  1442 |       0.911 |  1294 |      -0.124 |    -148 |
>
>
> Thanks,
> - Dave

-------------- next part --------------
A non-text attachment was scrubbed...
Name: new.patch
Type: text/x-patch
Size: 1677 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20110624/007e9b95/new.patch