On 3/14/2016 7:48 PM, Sundararajan Athijegannathan wrote:
Hi,
Thanks for the review. I've filed a bug to track the cleanups
suggested: https://bugs.openjdk.java.net/browse/JDK-8151860
Quick comments:
1) jrt fs is read-only file system as you noted. Copying content into
jrt fs does not make sense. Eventually read-only file system exception
is thrown.
But, yes that cast can be avoided.
Hi Sundar,
What I meant is without that "check&cast-ing", the copyToTarget might
actually work if the "dst/target" is a non-jrtfs-path", for example, copy
a "class" file out of the jrtfs and store into the local file system
(those code
originally is designed/implemented to work that way, whether or not it
works depends on if the src is readable and if the dst is writable). The
question here is if this is something we want to do for the jrtfs. That is
a design decision to make.
-Sherman
2) jrt-fs.jar is generated from jimage and jrt-fs sources and expected
to work on previous JDK (jdk 8 in this case). This is to support
cross-JDK reference to platform classes (from IDEs).
So, it is better to avoid internal JDK classes such as sun.nio.fs.Globs
in jrt-fs code.
Thanks,
-Sundar
On 3/15/2016 4:51 AM, Xueming Shen wrote:
(5) JrtFileSystemProvider.copy(src, dst, ...options)
The "dst/target" should not be cast to "AbstractJrtPath"? the jrtfs
itself is
a readonly, I'm not sure if we want to support the operation of copying
a "file" output jrtfs. The copy/paste "copyToTarget" appears to be
functional,
if it's not a "AbstractJrtPath".
On 03/14/2016 04:08 PM, Xueming Shen wrote:
(1) JrtFilePath: it does not seem to help performance to use the
byte[] as the
internal storage.
(2) AbstractJrtFilesystem.java
getPathMatcher:
....
if (syntax.equalsIgnoreCase(GLOB_SYNTAX)) {
expr = JrtUtils.toRegexPattern(input);
} else if (syntax.equalsIgnoreCase(REGEX_SYNTAX)) {
expr = input;
} else {
throw new UnsupportedOperationException("Syntax '" +
syntax
+ "' not recognized");
}
(3) can't JrtFileSystem use sun.nio.fs.Globs ? instead of its own copy?
(4) JrtFilesystem.nodesToIterator()
Stream has a "iterator() method. why need a collect() here?
different performance?
for example
private Iterator<Path> nodesToIterator(AbstractJrtPath dir,
List<Node> childNodes) {
return childNodes.stream()
.map(child ->
(Path)dir.resolve(toJrtPath(child.getNameString()).getFileName()))
.iterator();
}
sherman
