RFR 8061297 : sun/reflect/CallerSensitive/CallerSensitiveFinder.java should use the JRT FileSystem

[Chris Hegarty]

Alan, Paul,

Sometimes with these changes you do not want to cloud the significant 
body of the change with other refactoring. That said, we are on round 
two, so it seems reasonable.

I think I have captured/addressed all comments:

   http://cr.openjdk.java.net/~chegar/8061297/webrev.01/webrev/

-Chris.

On 14/01/15 14:32, Paul Sandoz wrote:
>
> On Jan 14, 2015, at 2:55 PM, Chris Hegarty <chris.hegarty at oracle.com> wrote:
>
>> This test currently completes successfully, but indicates that it has "Parsed 0 classfiles". It doesn't do anything useful.
>>
>> The modular image no longer contains jar files. The test should use the JRT FileSystem to iterate over the image classfiles.
>>
>> http://cr.openjdk.java.net/~chegar/8061297/webrev.00/webrev/
>>
>
>   191     static class PlatformClassPath {
>   192         static List<Path> getClasses() throws IOException {
>   193             List<Path> result = new ArrayList<Path>();
>   194             Path home = Paths.get(System.getProperty("java.home"));
>   195
>   196             if (home.resolve("lib").toFile().exists()) {
>   197                 // either an exploded build or an image
>   198                 File classes = home.resolve("modules").toFile();
>   199                 if (classes.exists() && classes.isDirectory()) {
>   200                     result.add(classes.toPath());
>   201                 } else {
>   202                     JrtClasses jrt = new JrtClasses();
>   203                     result.addAll(jrt.paths());
>   204                 }
>
> There is no need to copy the list. At line 200 one can return a singleton list.
>
> The list is copied yet again if there are no explicit classes added:
>
>    73         if (classes.isEmpty()) {
>    74             classes.addAll(PlatformClassPath.getClasses());
>    75         }
>
> You should be able to just copy the reference if classes is changed to List:
>
>    classes = PlatformClassPath.getClasses();
>
> (IIUC there is just one pass over the classes so you could probably modify to be totally lazy and process on the Stream directly but that would require slightly more modification to the run method)
>
>
>   230         private Stream<Path> toStream(Path root) {
>   231             try {
>   232                 return Files.walk(root);
>   233             } catch(IOException x) {
>   234                 x.printStackTrace(System.err);
>   235             }
>   236             return Collections.<Path>emptyList().stream();
>
> Is there a reason why the exception is swallowed? If not it would be better to wrap in an UncheckedIOException and re-throw.
>
> In general is this test thread safe if multiple tasks are submitted in a thread pool? i.e. can the visit method be called concurrently and update the non-synchronized csMethodsMissingAnnotation?
>
> Paul.
>