8150496: (zipfs) Fix performance issues in zip-fs
authorsherman
Fri, 06 May 2016 14:03:08 -0700
changeset 37803 ac955d7f5271
parent 37802 91fc9ac7b8b3
child 37804 3d485fe01521
child 37805 62e458968aad
child 37809 ff527b6dce32
8150496: (zipfs) Fix performance issues in zip-fs 8150366: (zipfs) lastAccessTime and createTime returned as null instead of default value Reviewed-by: alanb, redestad, shade
jdk/src/jdk.zipfs/share/classes/jdk/nio/zipfs/JarFileSystem.java
jdk/src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileAttributes.java
jdk/src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java
jdk/src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipPath.java
jdk/test/jdk/nio/zipfs/Basic.java
jdk/test/jdk/nio/zipfs/PathOps.java
--- a/jdk/src/jdk.zipfs/share/classes/jdk/nio/zipfs/JarFileSystem.java	Fri May 06 11:33:32 2016 -0700
+++ b/jdk/src/jdk.zipfs/share/classes/jdk/nio/zipfs/JarFileSystem.java	Fri May 06 14:03:08 2016 -0700
@@ -52,10 +52,10 @@
     private Function<byte[],byte[]> lookup;
 
     @Override
-    Entry getEntry(byte[] path) throws IOException {
+    IndexNode getInode(byte[] path) {
         // check for an alias to a versioned entry
         byte[] versionedPath = lookup.apply(path);
-        return versionedPath == null ? super.getEntry(path) : super.getEntry(versionedPath);
+        return versionedPath == null ? super.getInode(path) : super.getInode(versionedPath);
     }
 
     JarFileSystem(ZipFileSystemProvider provider, Path zfpath, Map<String,?> env)
@@ -87,7 +87,7 @@
     }
 
     private boolean isMultiReleaseJar() {
-        try (InputStream is = newInputStream(getBytes("META-INF/MANIFEST.MF"))) {
+        try (InputStream is = newInputStream(getBytes("/META-INF/MANIFEST.MF"))) {
             return (new Manifest(is)).getMainAttributes()
                     .containsKey(new Attributes.Name("Multi-Release"));
             // fixme change line above after JarFile integration to contain Attributes.Name.MULTI_RELEASE
@@ -108,7 +108,7 @@
      */
     private Function<byte[],byte[]> createVersionedLinks(int version) {
         HashMap<IndexNode,byte[]> aliasMap = new HashMap<>();
-        getVersionMap(version, getInode(getBytes("META-INF/versions"))).values()
+        getVersionMap(version, getInode(getBytes("/META-INF/versions"))).values()
                 .forEach(versionNode -> {   // for each META-INF/versions/{n} directory
                     // put all the leaf inodes, i.e. entries, into the alias map
                     // possibly shadowing lower versioned entries
@@ -176,7 +176,7 @@
      *   returns foo/bar.class
      */
     private byte[] getRootName(IndexNode prefix, IndexNode inode) {
-        int offset = prefix.name.length;
+        int offset = prefix.name.length - 1;
         byte[] fullName = inode.name;
         return Arrays.copyOfRange(fullName, offset, fullName.length);
     }
--- a/jdk/src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileAttributes.java	Fri May 06 11:33:32 2016 -0700
+++ b/jdk/src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileAttributes.java	Fri May 06 14:03:08 2016 -0700
@@ -26,122 +26,17 @@
 package jdk.nio.zipfs;
 
 import java.nio.file.attribute.BasicFileAttributes;
-import java.nio.file.attribute.FileTime;
-import java.util.Arrays;
-import java.util.Formatter;
-import static jdk.nio.zipfs.ZipUtils.*;
 
 /**
  *
  * @author  Xueming Shen, Rajendra Gutupalli,Jaya Hangal
  */
 
-class ZipFileAttributes implements BasicFileAttributes
-{
-    private final ZipFileSystem.Entry e;
-
-    ZipFileAttributes(ZipFileSystem.Entry e) {
-        this.e = e;
-    }
-
-    ///////// basic attributes ///////////
-    @Override
-    public FileTime creationTime() {
-        if (e.ctime != -1)
-            return FileTime.fromMillis(e.ctime);
-        return null;
-    }
-
-    @Override
-    public boolean isDirectory() {
-        return e.isDir();
-    }
-
-    @Override
-    public boolean isOther() {
-        return false;
-    }
-
-    @Override
-    public boolean isRegularFile() {
-        return !e.isDir();
-    }
-
-    @Override
-    public FileTime lastAccessTime() {
-        if (e.atime != -1)
-            return FileTime.fromMillis(e.atime);
-        return null;
-    }
-
-    @Override
-    public FileTime lastModifiedTime() {
-        return FileTime.fromMillis(e.mtime);
-    }
-
-    @Override
-    public long size() {
-        return e.size;
-    }
-
-    @Override
-    public boolean isSymbolicLink() {
-        return false;
-    }
-
-    @Override
-    public Object fileKey() {
-        return null;
-    }
-
-    ///////// zip entry attributes ///////////
-    public long compressedSize() {
-        return e.csize;
-    }
-
-    public long crc() {
-        return e.crc;
-    }
-
-    public int method() {
-        return e.method;
-    }
-
-    public byte[] extra() {
-        if (e.extra != null)
-            return Arrays.copyOf(e.extra, e.extra.length);
-        return null;
-    }
-
-    public byte[] comment() {
-        if (e.comment != null)
-            return Arrays.copyOf(e.comment, e.comment.length);
-        return null;
-    }
-
-    public String toString() {
-        StringBuilder sb = new StringBuilder(1024);
-        Formatter fm = new Formatter(sb);
-        if (creationTime() != null)
-            fm.format("    creationTime    : %tc%n", creationTime().toMillis());
-        else
-            fm.format("    creationTime    : null%n");
-
-        if (lastAccessTime() != null)
-            fm.format("    lastAccessTime  : %tc%n", lastAccessTime().toMillis());
-        else
-            fm.format("    lastAccessTime  : null%n");
-        fm.format("    lastModifiedTime: %tc%n", lastModifiedTime().toMillis());
-        fm.format("    isRegularFile   : %b%n", isRegularFile());
-        fm.format("    isDirectory     : %b%n", isDirectory());
-        fm.format("    isSymbolicLink  : %b%n", isSymbolicLink());
-        fm.format("    isOther         : %b%n", isOther());
-        fm.format("    fileKey         : %s%n", fileKey());
-        fm.format("    size            : %d%n", size());
-        fm.format("    compressedSize  : %d%n", compressedSize());
-        fm.format("    crc             : %x%n", crc());
-        fm.format("    method          : %d%n", method());
-        fm.close();
-        return sb.toString();
-    }
+interface ZipFileAttributes extends BasicFileAttributes {
+    public long compressedSize();
+    public long crc();
+    public int method();
+    public byte[] extra();
+    public byte[] comment();
+    public String toString();
 }
--- a/jdk/src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java	Fri May 06 11:33:32 2016 -0700
+++ b/jdk/src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java	Fri May 06 14:03:08 2016 -0700
@@ -68,36 +68,29 @@
 class ZipFileSystem extends FileSystem {
 
     private final ZipFileSystemProvider provider;
-    private final ZipPath defaultdir;
-    private boolean readOnly = false;
     private final Path zfpath;
     private final ZipCoder zc;
-
+    private final boolean noExtt;        // see readExtra()
+    private final ZipPath rootdir;
     // configurable by env map
-    private final String  defaultDir;    // default dir for the file system
-    private final String  nameEncoding;  // default encoding for name/comment
     private final boolean useTempFile;   // use a temp file for newOS, default
                                          // is to use BAOS for better performance
-    private final boolean createNew;     // create a new zip if not exists
+    private boolean readOnly = false;    // readonly file system
     private static final boolean isWindows = AccessController.doPrivileged(
             (PrivilegedAction<Boolean>) () -> System.getProperty("os.name")
                                                     .startsWith("Windows"));
 
     ZipFileSystem(ZipFileSystemProvider provider,
                   Path zfpath,
-                  Map<String, ?> env)
-        throws IOException
+                  Map<String, ?> env)  throws IOException
     {
-        // configurable env setup
-        this.createNew    = "true".equals(env.get("create"));
-        this.nameEncoding = env.containsKey("encoding") ?
-                            (String)env.get("encoding") : "UTF-8";
+        // create a new zip if not exists
+        boolean createNew = "true".equals(env.get("create"));
+        // default encoding for name/comment
+        String nameEncoding = env.containsKey("encoding") ?
+                              (String)env.get("encoding") : "UTF-8";
+        this.noExtt = "false".equals(env.get("zipinfo-time"));
         this.useTempFile  = TRUE.equals(env.get("useTempFile"));
-        this.defaultDir   = env.containsKey("default.dir") ?
-                            (String)env.get("default.dir") : "/";
-        if (this.defaultDir.charAt(0) != '/')
-            throw new IllegalArgumentException("default dir should be absolute");
-
         this.provider = provider;
         this.zfpath = zfpath;
         if (Files.notExists(zfpath)) {
@@ -113,10 +106,9 @@
         zfpath.getFileSystem().provider().checkAccess(zfpath, AccessMode.READ);
         boolean writeable = AccessController.doPrivileged(
             (PrivilegedAction<Boolean>) () ->  Files.isWritable(zfpath));
-        if (!writeable)
-            this.readOnly = true;
+        this.readOnly = !writeable;
         this.zc = ZipCoder.get(nameEncoding);
-        this.defaultdir = new ZipPath(this, getBytes(defaultDir));
+        this.rootdir = new ZipPath(this, new byte[]{'/'});
         this.ch = Files.newByteChannel(zfpath, READ);
         try {
             this.cen = initCEN();
@@ -161,33 +153,29 @@
 
     @Override
     public Iterable<Path> getRootDirectories() {
-        ArrayList<Path> pathArr = new ArrayList<>();
-        pathArr.add(new ZipPath(this, new byte[]{'/'}));
-        return pathArr;
+        return List.of(rootdir);
     }
 
-    ZipPath getDefaultDir() {  // package private
-        return defaultdir;
+    ZipPath getRootDir() {
+        return rootdir;
     }
 
     @Override
     public ZipPath getPath(String first, String... more) {
-        String path;
         if (more.length == 0) {
-            path = first;
-        } else {
-            StringBuilder sb = new StringBuilder();
-            sb.append(first);
-            for (String segment: more) {
-                if (segment.length() > 0) {
-                    if (sb.length() > 0)
-                        sb.append('/');
-                    sb.append(segment);
+            return new ZipPath(this, getBytes(first));
+        }
+        StringBuilder sb = new StringBuilder();
+        sb.append(first);
+        for (String path : more) {
+            if (path.length() > 0) {
+                if (sb.length() > 0) {
+                    sb.append('/');
                 }
+                sb.append(path);
             }
-            path = sb.toString();
         }
-        return new ZipPath(this, getBytes(path));
+        return new ZipPath(this, getBytes(sb.toString()));
     }
 
     @Override
@@ -206,14 +194,11 @@
 
     @Override
     public Iterable<FileStore> getFileStores() {
-        ArrayList<FileStore> list = new ArrayList<>(1);
-        list.add(new ZipFileStore(new ZipPath(this, new byte[]{'/'})));
-        return list;
+        return List.of(new ZipFileStore(rootdir));
     }
 
     private static final Set<String> supportedFileAttributeViews =
-            Collections.unmodifiableSet(
-                new HashSet<String>(Arrays.asList("basic", "zip")));
+            Set.of("basic", "zip");
 
     @Override
     public Set<String> supportedFileAttributeViews() {
@@ -331,12 +316,26 @@
                     return null;
                 e = new Entry(inode.name);       // pseudo directory
                 e.method = METHOD_STORED;        // STORED for dir
-                e.mtime = e.atime = e.ctime = -1;// -1 for all times
+                e.mtime = e.atime = e.ctime = zfsDefaultTimeStamp;
             }
         } finally {
             endRead();
         }
-        return new ZipFileAttributes(e);
+        return e;
+    }
+
+    void checkAccess(byte[] path) throws IOException {
+        beginRead();
+        try {
+            ensureOpen();
+            // is it necessary to readCEN as a sanity check?
+            if (getInode(path) == null) {
+                throw new NoSuchFileException(toString());
+            }
+
+        } finally {
+            endRead();
+        }
     }
 
     void setTimes(byte[] path, FileTime mtime, FileTime atime, FileTime ctime)
@@ -387,14 +386,6 @@
         }
     }
 
-    private ZipPath toZipPath(byte[] path) {
-        // make it absolute
-        byte[] p = new byte[path.length + 1];
-        p[0] = '/';
-        System.arraycopy(path, 0, p, 1, path.length);
-        return new ZipPath(this, p);
-    }
-
     // returns the list of child paths of "path"
     Iterator<Path> iteratorOf(byte[] path,
                               DirectoryStream.Filter<? super Path> filter)
@@ -409,7 +400,7 @@
             List<Path> list = new ArrayList<>();
             IndexNode child = inode.child;
             while (child != null) {
-                ZipPath zp = toZipPath(child.name);
+                ZipPath zp = new ZipPath(this, child.name);
                 if (filter == null || filter.accept(zp))
                     list.add(zp);
                 child = child.sibling;
@@ -450,6 +441,7 @@
         try {
             ensureOpen();
             Entry eSrc = getEntry(src);  // ensureOpen checked
+
             if (eSrc == null)
                 throw new NoSuchFileException(getString(src));
             if (eSrc.isDir()) {    // spec says to create dst dir
@@ -679,7 +671,7 @@
                     }
 
                     public SeekableByteChannel truncate(long size)
-                    throws IOException
+                        throws IOException
                     {
                         throw new NonWritableChannelException();
                     }
@@ -879,7 +871,8 @@
     private void checkParents(byte[] path) throws IOException {
         beginRead();
         try {
-            while ((path = getParent(path)) != null && path.length != 0) {
+            while ((path = getParent(path)) != null &&
+                    path != ROOTPATH) {
                 if (!inodes.containsKey(IndexNode.keyOf(path))) {
                     throw new NoSuchFileException(getString(path));
                 }
@@ -889,15 +882,20 @@
         }
     }
 
-    private static byte[] ROOTPATH = new byte[0];
+    private static byte[] ROOTPATH = new byte[] { '/' };
     private static byte[] getParent(byte[] path) {
+        int off = getParentOff(path);
+        if (off <= 1)
+            return ROOTPATH;
+        return Arrays.copyOf(path, off + 1);
+    }
+
+    private static int getParentOff(byte[] path) {
         int off = path.length - 1;
         if (off > 0 && path[off] == '/')  // isDirectory
             off--;
         while (off > 0 && path[off] != '/') { off--; }
-        if (off <= 0)
-            return ROOTPATH;
-        return Arrays.copyOf(path, off + 1);
+        return off;
     }
 
     private final void beginWrite() {
@@ -941,19 +939,6 @@
         close();
     }
 
-    private long getDataPos(Entry e) throws IOException {
-        if (e.locoff == -1) {
-            Entry e2 = getEntry(e.name);
-            if (e2 == null)
-                throw new ZipException("invalid loc for entry <" + e.name + ">");
-            e.locoff = e2.locoff;
-        }
-        byte[] buf = new byte[LOCHDR];
-        if (readFullyAt(buf, 0, buf.length, e.locoff) != buf.length)
-            throw new ZipException("invalid loc for entry <" + e.name + ">");
-        return locpos + e.locoff + LOCHDR + LOCNAM(buf) + LOCEXT(buf);
-    }
-
     // Reads len bytes of data from the specified offset into buf.
     // Returns the total number of bytes read.
     // Each/every byte read from here (except the cen, which is mapped).
@@ -1090,7 +1075,9 @@
             if (pos + CENHDR + nlen > limit) {
                 zerror("invalid CEN header (bad header size)");
             }
-            byte[] name = Arrays.copyOfRange(cen, pos + CENHDR, pos + CENHDR + nlen);
+            byte[] name = new byte[nlen + 1];
+            System.arraycopy(cen, pos + CENHDR, name, 1, nlen);
+            name[0] = '/';
             IndexNode inode = new IndexNode(name, pos);
             inodes.put(inode, inode);
             // skip ext and comment
@@ -1278,7 +1265,7 @@
                     if (inode.pos == -1) {
                         continue;               // pseudo directory node
                     }
-                    e = Entry.readCEN(this, inode.pos);
+                    e = Entry.readCEN(this, inode);
                     try {
                         written += copyLOCEntry(e, false, os, written, buf);
                         elist.add(e);
@@ -1320,13 +1307,6 @@
 
         Files.move(tmpFile, zfpath, REPLACE_EXISTING);
         hasUpdate = false;    // clear
-        /*
-        if (isOpen) {
-            ch = zfpath.newByteChannel(READ); // re-fresh "ch" and "cen"
-            cen = initCEN();
-        }
-         */
-        //System.out.printf("->sync(%s) done!%n", toString());
     }
 
     IndexNode getInode(byte[] path) {
@@ -1350,7 +1330,7 @@
             return (Entry)inode;
         if (inode == null || inode.pos == -1)
             return null;
-        return Entry.readCEN(this, inode.pos);
+        return Entry.readCEN(this, inode);
     }
 
     public void deleteFile(byte[] path, boolean failIfNotExists)
@@ -1432,7 +1412,6 @@
                 bufSize = 8192;
             final long size = e.size;
             eis = new InflaterInputStream(eis, getInflater(), (int)bufSize) {
-
                 private boolean isClosed = false;
                 public void close() throws IOException {
                     if (!isClosed) {
@@ -1493,10 +1472,20 @@
             this.zfch = zfch;
             rem = e.csize;
             size = e.size;
-            pos = getDataPos(e);
+            pos = e.locoff;
+            if (pos == -1) {
+                Entry e2 = getEntry(e.name);
+                if (e2 == null) {
+                    throw new ZipException("invalid loc for entry <" + e.name + ">");
+                }
+                pos = e2.locoff;
+            }
+            pos = -pos;  // lazy initialize the real data offset
         }
+
         public int read(byte b[], int off, int len) throws IOException {
             ensureOpen();
+            initDataPos();
             if (rem == 0) {
                 return -1;
             }
@@ -1523,6 +1512,7 @@
             }
             return (int)n;
         }
+
         public int read() throws IOException {
             byte[] b = new byte[1];
             if (read(b, 0, 1) == 1) {
@@ -1531,6 +1521,7 @@
                 return -1;
             }
         }
+
         public long skip(long n) throws IOException {
             ensureOpen();
             if (n > rem)
@@ -1542,16 +1533,30 @@
             }
             return n;
         }
+
         public int available() {
             return rem > Integer.MAX_VALUE ? Integer.MAX_VALUE : (int) rem;
         }
+
         public long size() {
             return size;
         }
+
         public void close() {
             rem = 0;
             streams.remove(this);
         }
+
+        private void initDataPos() throws IOException {
+            if (pos <= 0) {
+                pos = -pos + locpos;
+                byte[] buf = new byte[LOCHDR];
+                if (readFullyAt(buf, 0, buf.length, pos) != LOCHDR) {
+                    throw new ZipException("invalid loc " + pos + " for entry reading");
+                }
+                pos += LOCHDR + LOCNAM(buf) + LOCEXT(buf);
+            }
+        }
     }
 
     class EntryOutputStream extends DeflaterOutputStream
@@ -1701,8 +1706,9 @@
 
     // End of central directory record
     static class END {
-        int  disknum;
-        int  sdisknum;
+        // these 2 fields are not used by anyone and write() uses "0"
+        // int  disknum;
+        // int  sdisknum;
         int  endsub;     // endsub
         int  centot;     // 4 bytes
         long cenlen;     // 4 bytes
@@ -1711,9 +1717,9 @@
         byte[] comment;
 
         /* members of Zip64 end of central directory locator */
-        int diskNum;
+        // int diskNum;
         long endpos;
-        int disktot;
+        // int disktot;
 
         void write(OutputStream os, long offset) throws IOException {
             boolean hasZip64 = false;
@@ -1776,6 +1782,11 @@
         int    hashcode;  // node is hashable/hashed by its name
         int    pos = -1;  // position in cen table, -1 menas the
                           // entry does not exists in zip file
+        IndexNode(byte[] name) {
+            name(name);
+            this.pos = -1;
+        }
+
         IndexNode(byte[] name, int pos) {
             name(name);
             this.pos = pos;
@@ -1804,6 +1815,9 @@
             if (!(other instanceof IndexNode)) {
                 return false;
             }
+            if (other instanceof ParentLookup) {
+                return ((ParentLookup)other).equals(this);
+            }
             return Arrays.equals(name, ((IndexNode)other).name);
         }
 
@@ -1816,17 +1830,16 @@
         IndexNode child;  // 1st child
     }
 
-    static class Entry extends IndexNode {
+    static class Entry extends IndexNode implements ZipFileAttributes {
 
-        static final int CEN    = 1;    // entry read from cen
-        static final int NEW    = 2;    // updated contents in bytes or file
-        static final int FILECH = 3;    // fch update in "file"
-        static final int COPY   = 4;    // copy of a CEN entry
+        static final int CEN    = 1;  // entry read from cen
+        static final int NEW    = 2;  // updated contents in bytes or file
+        static final int FILECH = 3;  // fch update in "file"
+        static final int COPY   = 4;  // copy of a CEN entry
 
-
-        byte[] bytes;      // updated content bytes
-        Path   file;       // use tmp file to store bytes;
-        int    type = CEN; // default is the entry read from cen
+        byte[] bytes;                 // updated content bytes
+        Path   file;                  // use tmp file to store bytes;
+        int    type = CEN;            // default is the entry read from cen
 
         // entry attributes
         int    version;
@@ -1841,10 +1854,12 @@
         byte[] extra;
 
         // cen
-        int    versionMade;
-        int    disk;
-        int    attrs;
-        long   attrsEx;
+
+        // these fields are not used by anyone and writeCEN uses "0"
+        // int    versionMade;
+        // int    disk;
+        // int    attrs;
+        // long   attrsEx;
         long   locoff;
         byte[] comment;
 
@@ -1875,10 +1890,12 @@
             this.csize     = e.csize;
             this.method    = e.method;
             this.extra     = e.extra;
+            /*
             this.versionMade = e.versionMade;
             this.disk      = e.disk;
             this.attrs     = e.attrs;
             this.attrsEx   = e.attrsEx;
+            */
             this.locoff    = e.locoff;
             this.comment   = e.comment;
             this.type      = type;
@@ -1899,19 +1916,19 @@
         }
 
         ///////////////////// CEN //////////////////////
-        static Entry readCEN(ZipFileSystem zipfs, int pos)
+        static Entry readCEN(ZipFileSystem zipfs, IndexNode inode)
             throws IOException
         {
-            return new Entry().cen(zipfs, pos);
+            return new Entry().cen(zipfs, inode);
         }
 
-        private Entry cen(ZipFileSystem zipfs, int pos)
+        private Entry cen(ZipFileSystem zipfs, IndexNode inode)
             throws IOException
         {
             byte[] cen = zipfs.cen;
+            int pos = inode.pos;
             if (!cenSigAt(cen, pos))
                 zerror("invalid CEN header (bad signature)");
-            versionMade = CENVEM(cen, pos);
             version     = CENVER(cen, pos);
             flag        = CENFLG(cen, pos);
             method      = CENHOW(cen, pos);
@@ -1922,13 +1939,16 @@
             int nlen    = CENNAM(cen, pos);
             int elen    = CENEXT(cen, pos);
             int clen    = CENCOM(cen, pos);
+            /*
+            versionMade = CENVEM(cen, pos);
             disk        = CENDSK(cen, pos);
             attrs       = CENATT(cen, pos);
             attrsEx     = CENATX(cen, pos);
+            */
             locoff      = CENOFF(cen, pos);
-
             pos += CENHDR;
-            name(Arrays.copyOfRange(cen, pos, pos + nlen));
+            this.name = inode.name;
+            this.hashcode = inode.hashcode;
 
             pos += nlen;
             if (elen > 0) {
@@ -1955,7 +1975,8 @@
             boolean foundExtraTime = false;  // if time stamp NTFS, EXTT present
 
             // confirm size/length
-            int nlen = (name != null) ? name.length : 0;
+
+            int nlen = (name != null) ? name.length - 1 : 0;  // name has [0] as "slash"
             int elen = (extra != null) ? extra.length : 0;
             int eoff = 0;
             int clen = (comment != null) ? comment.length : 0;
@@ -2004,7 +2025,7 @@
             writeInt(os, crc);               // crc-32
             writeInt(os, csize0);            // compressed size
             writeInt(os, size0);             // uncompressed size
-            writeShort(os, name.length);
+            writeShort(os, nlen);
             writeShort(os, elen + elen64 + elenNTFS + elenEXTT);
 
             if (comment != null) {
@@ -2016,7 +2037,7 @@
             writeShort(os, 0);              // internal file attributes (unused)
             writeInt(os, 0);                // external file attributes (unused)
             writeInt(os, locoff0);          // relative offset of local header
-            writeBytes(os, name);
+            writeBytes(os, name, 1, nlen);
             if (elen64 != 0) {
                 writeShort(os, EXTID_ZIP64);// Zip64 extra
                 writeShort(os, elen64 - 4); // size of "this" extra block
@@ -2086,8 +2107,9 @@
             int nlen = LOCNAM(buf);
             int elen = LOCEXT(buf);
 
-            name = new byte[nlen];
-            if (zipfs.readFullyAt(name, 0, nlen, pos + LOCHDR) != nlen) {
+            name = new byte[nlen + 1];
+            name[0] = '/';
+            if (zipfs.readFullyAt(name, 1, nlen, pos + LOCHDR) != nlen) {
                 throw new ZipException("loc: name reading failed");
             }
             if (elen > 0) {
@@ -2130,12 +2152,10 @@
             return this;
         }
 
-        int writeLOC(OutputStream os)
-            throws IOException
-        {
+        int writeLOC(OutputStream os) throws IOException {
             writeInt(os, LOCSIG);               // LOC header signature
             int version = version();
-            int nlen = (name != null) ? name.length : 0;
+            int nlen = (name != null) ? name.length - 1 : 0; // [0] is slash
             int elen = (extra != null) ? extra.length : 0;
             boolean foundExtraTime = false;     // if extra timestamp present
             int eoff = 0;
@@ -2192,9 +2212,9 @@
                         elenEXTT += 4;
                 }
             }
-            writeShort(os, name.length);
+            writeShort(os, nlen);
             writeShort(os, elen + elen64 + elenNTFS + elenEXTT);
-            writeBytes(os, name);
+            writeBytes(os, name, 1, nlen);
             if (elen64 != 0) {
                 writeShort(os, EXTID_ZIP64);
                 writeShort(os, 16);
@@ -2229,13 +2249,11 @@
             if (extra != null) {
                 writeBytes(os, extra);
             }
-            return LOCHDR + name.length + elen + elen64 + elenNTFS + elenEXTT;
+            return LOCHDR + nlen + elen + elen64 + elenNTFS + elenEXTT;
         }
 
         // Data Descriptior
-        int writeEXT(OutputStream os)
-            throws IOException
-        {
+        int writeEXT(OutputStream os) throws IOException {
             writeInt(os, EXTSIG);           // EXT header signature
             writeInt(os, crc);              // crc-32
             if (csize >= ZIP64_MINVAL || size >= ZIP64_MINVAL) {
@@ -2301,7 +2319,15 @@
                     break;
                 case EXTID_EXTT:
                     // spec says the Extened timestamp in cen only has mtime
-                    // need to read the loc to get the extra a/ctime
+                    // need to read the loc to get the extra a/ctime, if flag
+                    // "zipinfo-time" is not specified to false;
+                    // there is performance cost (move up to loc and read) to
+                    // access the loc table foreach entry;
+                    if (zipfs.noExtt) {
+                        if (sz == 5)
+                            mtime = unixToJavaTime(LG(extra, pos + 1));
+                         break;
+                    }
                     byte[] buf = new byte[LOCHDR];
                     if (zipfs.readFullyAt(buf, 0, buf.length , locoff)
                         != buf.length)
@@ -2309,7 +2335,6 @@
                     if (!locSigAt(buf, 0))
                         throw new ZipException("loc: wrong sig ->"
                                            + Long.toString(getSig(buf, 0), 16));
-
                     int locElen = LOCEXT(buf);
                     if (locElen < 9)    // EXTT is at lease 9 bytes
                         break;
@@ -2354,6 +2379,96 @@
             else
                 extra = null;
         }
+
+        ///////// basic file attributes ///////////
+        @Override
+        public FileTime creationTime() {
+            return FileTime.fromMillis(ctime == -1 ? mtime : ctime);
+        }
+
+        @Override
+        public boolean isDirectory() {
+            return isDir();
+        }
+
+        @Override
+        public boolean isOther() {
+            return false;
+        }
+
+        @Override
+        public boolean isRegularFile() {
+            return !isDir();
+        }
+
+        @Override
+        public FileTime lastAccessTime() {
+            return FileTime.fromMillis(atime == -1 ? mtime : atime);
+        }
+
+        @Override
+        public FileTime lastModifiedTime() {
+            return FileTime.fromMillis(mtime);
+        }
+
+        @Override
+        public long size() {
+            return size;
+        }
+
+        @Override
+        public boolean isSymbolicLink() {
+            return false;
+        }
+
+        @Override
+        public Object fileKey() {
+            return null;
+        }
+
+        ///////// zip entry attributes ///////////
+        public long compressedSize() {
+            return csize;
+        }
+
+        public long crc() {
+            return crc;
+        }
+
+        public int method() {
+            return method;
+        }
+
+        public byte[] extra() {
+            if (extra != null)
+                return Arrays.copyOf(extra, extra.length);
+            return null;
+        }
+
+        public byte[] comment() {
+            if (comment != null)
+                return Arrays.copyOf(comment, comment.length);
+            return null;
+        }
+
+        public String toString() {
+            StringBuilder sb = new StringBuilder(1024);
+            Formatter fm = new Formatter(sb);
+            fm.format("    creationTime    : %tc%n", creationTime().toMillis());
+            fm.format("    lastAccessTime  : %tc%n", lastAccessTime().toMillis());
+            fm.format("    lastModifiedTime: %tc%n", lastModifiedTime().toMillis());
+            fm.format("    isRegularFile   : %b%n", isRegularFile());
+            fm.format("    isDirectory     : %b%n", isDirectory());
+            fm.format("    isSymbolicLink  : %b%n", isSymbolicLink());
+            fm.format("    isOther         : %b%n", isOther());
+            fm.format("    fileKey         : %s%n", fileKey());
+            fm.format("    size            : %d%n", size());
+            fm.format("    compressedSize  : %d%n", compressedSize());
+            fm.format("    crc             : %x%n", crc());
+            fm.format("    method          : %d%n", method());
+            fm.close();
+            return sb.toString();
+        }
     }
 
     private static class ExChannelCloser  {
@@ -2379,25 +2494,8 @@
     // implemented below.
     private IndexNode root;
 
-    private void addToTree(IndexNode inode, HashSet<IndexNode> dirs) {
-        if (dirs.contains(inode)) {
-            return;
-        }
-        IndexNode parent;
-        byte[] name = inode.name;
-        byte[] pname = getParent(name);
-        if (inodes.containsKey(LOOKUPKEY.as(pname))) {
-            parent = inodes.get(LOOKUPKEY);
-        } else {    // pseudo directory entry
-            parent = new IndexNode(pname, -1);
-            inodes.put(parent, parent);
-        }
-        addToTree(parent, dirs);
-        inode.sibling = parent.child;
-        parent.child = inode;
-        if (name[name.length -1] == '/')
-            dirs.add(inode);
-    }
+    // default time stamp for pseudo entries
+    private long zfsDefaultTimeStamp = System.currentTimeMillis();
 
     private void removeFromTree(IndexNode inode) {
         IndexNode parent = inodes.get(LOOKUPKEY.as(getParent(inode.name)));
@@ -2417,15 +2515,69 @@
         }
     }
 
+    // purely for parent lookup, so we don't have to copy the parent
+    // name every time
+    static class ParentLookup extends IndexNode {
+        int len;
+        ParentLookup() {}
+
+        final ParentLookup as(byte[] name, int len) { // as a lookup "key"
+            name(name, len);
+            return this;
+        }
+
+        void name(byte[] name, int len) {
+            this.name = name;
+            this.len = len;
+            // calculate the hashcode the same way as Arrays.hashCode() does
+            int result = 1;
+            for (int i = 0; i < len; i++)
+                result = 31 * result + name[i];
+            this.hashcode = result;
+        }
+
+        @Override
+        public boolean equals(Object other) {
+            if (!(other instanceof IndexNode)) {
+                return false;
+            }
+            byte[] oname = ((IndexNode)other).name;
+            return Arrays.equals(name, 0, len,
+                                 oname, 0, oname.length);
+        }
+
+    }
+
     private void buildNodeTree() throws IOException {
         beginWrite();
         try {
-            HashSet<IndexNode> dirs = new HashSet<>();
-            IndexNode root = new IndexNode(ROOTPATH, -1);
+            IndexNode root = new IndexNode(ROOTPATH);
+            IndexNode[] nodes = inodes.keySet().toArray(new IndexNode[0]);
             inodes.put(root, root);
-            dirs.add(root);
-            for (IndexNode node : inodes.keySet().toArray(new IndexNode[0])) {
-                addToTree(node, dirs);
+            ParentLookup lookup = new ParentLookup();
+            for (IndexNode node : nodes) {
+                IndexNode parent;
+                while (true) {
+                    int off = getParentOff(node.name);
+                    if (off <= 1) {    // parent is root
+                        node.sibling = root.child;
+                        root.child = node;
+                        break;
+                    }
+                    lookup = lookup.as(node.name, off + 1);
+                    if (inodes.containsKey(lookup)) {
+                        parent = inodes.get(lookup);
+                        node.sibling = parent.child;
+                        parent.child = node;
+                        break;
+                    }
+                    // add new pseudo directory entry
+                    parent = new IndexNode(Arrays.copyOf(node.name, off + 1));
+                    inodes.put(parent, parent);
+                    node.sibling = parent.child;
+                    parent.child = node;
+                    node = parent;
+                }
             }
         } finally {
             endWrite();
--- a/jdk/src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipPath.java	Fri May 06 11:33:32 2016 -0700
+++ b/jdk/src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipPath.java	Fri May 06 14:03:08 2016 -0700
@@ -41,7 +41,7 @@
  * @author  Xueming Shen, Rajendra Gutupalli,Jaya Hangal
  */
 
-class ZipPath implements Path {
+final class ZipPath implements Path {
 
     private final ZipFileSystem zfs;
     private final byte[] path;
@@ -64,7 +64,7 @@
     @Override
     public ZipPath getRoot() {
         if (this.isAbsolute())
-            return new ZipPath(zfs, new byte[]{path[0]});
+            return zfs.getRootDir();
         else
             return null;
     }
@@ -145,7 +145,15 @@
 
     @Override
     public ZipPath toRealPath(LinkOption... options) throws IOException {
-        ZipPath realPath = new ZipPath(zfs, getResolvedPath()).toAbsolutePath();
+        ZipPath realPath;
+        byte[] resolved = getResolvedPath();
+        // resolved is always absolute and normalized
+        if (resolved == path) {
+            realPath = this;
+        } else {
+            realPath = new ZipPath(zfs, resolved, true);
+            realPath.resolved = resolved;
+        }
         realPath.checkAccess();
         return realPath;
     }
@@ -159,20 +167,11 @@
         if (isAbsolute()) {
             return this;
         } else {
-            //add / bofore the existing path
-            byte[] defaultdir = zfs.getDefaultDir().path;
-            int defaultlen = defaultdir.length;
-            boolean endsWith = (defaultdir[defaultlen - 1] == '/');
-            byte[] t = null;
-            if (endsWith)
-                t = new byte[defaultlen + path.length];
-            else
-                t = new byte[defaultlen + 1 + path.length];
-            System.arraycopy(defaultdir, 0, t, 0, defaultlen);
-            if (!endsWith)
-                t[defaultlen++] = '/';
-            System.arraycopy(path, 0, t, defaultlen, path.length);
-            return new ZipPath(zfs, t, true);  // normalized
+            // add '/' before the existing path
+            byte[] tmp = new byte[path.length + 1];
+            System.arraycopy(path, 0, tmp, 1, path.length);
+            tmp[0] = '/';
+            return new ZipPath(zfs, tmp, true);  // normalized
         }
     }
 
@@ -217,11 +216,15 @@
     public Path relativize(Path other) {
         final ZipPath o = checkPath(other);
         if (o.equals(this))
-            return new ZipPath(getFileSystem(), new byte[0], true);
-        if (/* this.getFileSystem() != o.getFileSystem() || */
-            this.isAbsolute() != o.isAbsolute()) {
+            return new ZipPath(zfs, new byte[0], true);
+        if (this.path.length == 0)
+            return o;
+        if (this.zfs != o.zfs || this.isAbsolute() != o.isAbsolute())
             throw new IllegalArgumentException();
-        }
+        if (this.path.length == 1 && this.path[0] == '/')
+            return new ZipPath(zfs,
+                               Arrays.copyOfRange(o.path, 1, o.path.length),
+                               true);
         int mc = this.getNameCount();
         int oc = o.getNameCount();
         int n = Math.min(mc, oc);
@@ -249,7 +252,7 @@
             System.arraycopy(o.path, o.offsets[i],
                              result, pos,
                              o.path.length - o.offsets[i]);
-        return new ZipPath(getFileSystem(), result);
+        return new ZipPath(zfs, result);
     }
 
     @Override
@@ -265,26 +268,29 @@
     @Override
     public ZipPath resolve(Path other) {
         final ZipPath o = checkPath(other);
-        if (o.isAbsolute())
+        int tlen = this.path.length;
+        if (tlen == 0 || o.isAbsolute())
             return o;
+        int olen = o.path.length;
+        if (olen == 0)
+            return this;
         byte[] resolved = null;
-        if (this.path[path.length - 1] == '/') {
-            resolved = new byte[path.length + o.path.length];
-            System.arraycopy(path, 0, resolved, 0, path.length);
-            System.arraycopy(o.path, 0, resolved, path.length, o.path.length);
+        if (this.path[tlen - 1] == '/') {
+            resolved = new byte[tlen + olen];
+            System.arraycopy(path, 0, resolved, 0, tlen);
+            System.arraycopy(o.path, 0, resolved, tlen, olen);
         } else {
-            resolved = new byte[path.length + 1 + o.path.length];
-            System.arraycopy(path, 0, resolved, 0, path.length);
-            resolved[path.length] = '/';
-            System.arraycopy(o.path, 0, resolved, path.length + 1, o.path.length);
+            resolved = new byte[tlen + 1 + olen];
+            System.arraycopy(path, 0, resolved, 0, tlen);
+            resolved[tlen] = '/';
+            System.arraycopy(o.path, 0, resolved, tlen + 1, olen);
         }
         return new ZipPath(zfs, resolved);
     }
 
     @Override
     public Path resolveSibling(Path other) {
-        if (other == null)
-            throw new NullPointerException();
+        Objects.requireNonNull(other, "other");
         Path parent = getParent();
         return (parent == null) ? other : parent.resolve(other);
     }
@@ -330,22 +336,22 @@
 
     @Override
     public ZipPath resolve(String other) {
-        return resolve(getFileSystem().getPath(other));
+        return resolve(zfs.getPath(other));
     }
 
     @Override
     public final Path resolveSibling(String other) {
-        return resolveSibling(getFileSystem().getPath(other));
+        return resolveSibling(zfs.getPath(other));
     }
 
     @Override
     public final boolean startsWith(String other) {
-        return startsWith(getFileSystem().getPath(other));
+        return startsWith(zfs.getPath(other));
     }
 
     @Override
     public final boolean endsWith(String other) {
-        return endsWith(getFileSystem().getPath(other));
+        return endsWith(zfs.getPath(other));
     }
 
     @Override
@@ -357,8 +363,7 @@
     }
 
     private ZipPath checkPath(Path path) {
-        if (path == null)
-            throw new NullPointerException();
+        Objects.requireNonNull(path, "path");
         if (!(path instanceof ZipPath))
             throw new ProviderMismatchException();
         return (ZipPath) path;
@@ -410,8 +415,6 @@
                 r = getResolved();
             else
                 r = toAbsolutePath().getResolvedPath();
-            if (r[0] == '/')
-                r = Arrays.copyOfRange(r, 1, r.length);
             resolved = r;
         }
         return resolved;
@@ -425,13 +428,10 @@
         byte prevC = 0;
         for (int i = 0; i < path.length; i++) {
             byte c = path[i];
-            if (c == '\\')
+            if (c == '\\' || c == '\u0000')
                 return normalize(path, i);
             if (c == (byte)'/' && prevC == '/')
                 return normalize(path, i - 1);
-            if (c == '\u0000')
-                throw new InvalidPathException(zfs.getString(path),
-                                               "Path: nul character not allowed");
             prevC = c;
         }
         return path;
@@ -465,12 +465,10 @@
 
     // Remove DotSlash(./) and resolve DotDot (..) components
     private byte[] getResolved() {
-        if (path.length == 0)
-            return path;
         for (int i = 0; i < path.length; i++) {
-            byte c = path[i];
-            if (c == (byte)'.')
+            if (path[i] == (byte)'.') {
                 return resolve0();
+            }
         }
         return path;
     }
@@ -612,7 +610,6 @@
 
     /////////////////////////////////////////////////////////////////////
 
-
     void createDirectory(FileAttribute<?>... attrs)
         throws IOException
     {
@@ -749,20 +746,13 @@
                     throw new UnsupportedOperationException();
             }
         }
-        ZipFileAttributes attrs = zfs.getFileAttributes(getResolvedPath());
-        if (attrs == null && (path.length != 1 || path[0] != '/'))
-            throw new NoSuchFileException(toString());
-        if (w) {
-            if (zfs.isReadOnly())
-                throw new AccessDeniedException(toString());
+        zfs.checkAccess(getResolvedPath());
+        if ((w && zfs.isReadOnly()) || x) {
+            throw new AccessDeniedException(toString());
         }
-        if (x)
-            throw new AccessDeniedException(toString());
     }
 
     boolean exists() {
-        if (path.length == 1 && path[0] == '/')
-            return true;
         try {
             return zfs.exists(getResolvedPath());
         } catch (IOException x) {}
--- a/jdk/test/jdk/nio/zipfs/Basic.java	Fri May 06 11:33:32 2016 -0700
+++ b/jdk/test/jdk/nio/zipfs/Basic.java	Fri May 06 14:03:08 2016 -0700
@@ -42,7 +42,7 @@
 
 /**
  * @test
- * @bug 8038500 8040059
+ * @bug 8038500 8040059 8150366 8150496
  * @summary Basic test for zip provider
  *
  * @run main Basic
@@ -155,7 +155,8 @@
             indent();
             System.out.print(file.getFileName());
             if (attrs.isRegularFile())
-                System.out.format(" (%d)", attrs.size());
+                System.out.format("%n%s%n", attrs);
+
             System.out.println();
             return FileVisitResult.CONTINUE;
         }
--- a/jdk/test/jdk/nio/zipfs/PathOps.java	Fri May 06 11:33:32 2016 -0700
+++ b/jdk/test/jdk/nio/zipfs/PathOps.java	Fri May 06 14:03:08 2016 -0700
@@ -47,15 +47,15 @@
     private Path path;
     private Exception exc;
 
-    private PathOps(String s) {
+    private PathOps(String first, String... more) {
         out.println();
-        input = s;
+        input = first;
         try {
-            path = fs.getPath(s);
-            out.format("%s -> %s", s, path);
+            path = fs.getPath(first, more);
+            out.format("%s -> %s", first, path);
         } catch (Exception x) {
             exc = x;
-            out.format("%s -> %s", s, x);
+            out.format("%s -> %s", first, x);
         }
         out.println();
     }
@@ -179,6 +179,13 @@
         return this;
     }
 
+    PathOps resolveSibling(String other, String expected) {
+        out.format("test resolveSibling %s\n", other);
+        checkPath();
+        check(path.resolveSibling(other), expected);
+        return this;
+    }
+
     PathOps relativize(String other, String expected) {
         out.format("test relativize %s\n", other);
         checkPath();
@@ -224,6 +231,10 @@
         return new PathOps(s);
     }
 
+    static PathOps test(String first, String... more) {
+        return new PathOps(first, more);
+    }
+
     // -- PathOpss --
 
     static void header(String s) {
@@ -235,6 +246,26 @@
     static void doPathOpTests() {
         header("Path operations");
 
+        // construction
+        test("/")
+            .string("/");
+        test("/", "")
+            .string("/");
+        test("/", "foo")
+            .string("/foo");
+        test("/", "/foo")
+            .string("/foo");
+        test("/", "foo/")
+            .string("/foo");
+        test("foo", "bar", "gus")
+            .string("foo/bar/gus");
+        test("")
+            .string("");
+        test("", "/")
+            .string("/");
+        test("", "foo", "", "bar", "", "/gus")
+            .string("foo/bar/gus");
+
         // all components
         test("/a/b/c")
             .root("/")
@@ -323,7 +354,6 @@
             .ends("foo/bar/")
             .ends("foo/bar");
 
-
         // elements
         test("a/b/c")
             .element(0,"a")
@@ -343,16 +373,57 @@
         // resolve
         test("/tmp")
             .resolve("foo", "/tmp/foo")
-            .resolve("/foo", "/foo");
+            .resolve("/foo", "/foo")
+            .resolve("", "/tmp");
         test("tmp")
             .resolve("foo", "tmp/foo")
+            .resolve("/foo", "/foo")
+            .resolve("", "tmp");
+        test("")
+            .resolve("", "")
+            .resolve("foo", "foo")
             .resolve("/foo", "/foo");
 
+        // resolveSibling
+        test("foo")
+            .resolveSibling("bar", "bar")
+            .resolveSibling("/bar", "/bar")
+            .resolveSibling("", "");
+        test("foo/bar")
+            .resolveSibling("gus", "foo/gus")
+            .resolveSibling("/gus", "/gus")
+            .resolveSibling("", "foo");
+        test("/foo")
+            .resolveSibling("gus", "/gus")
+            .resolveSibling("/gus", "/gus")
+            .resolveSibling("", "/");
+        test("/foo/bar")
+            .resolveSibling("gus", "/foo/gus")
+            .resolveSibling("/gus", "/gus")
+            .resolveSibling("", "/foo");
+        test("")
+            .resolveSibling("foo", "foo")
+            .resolveSibling("/foo", "/foo")
+            .resolve("", "");
+
         // relativize
         test("/a/b/c")
             .relativize("/a/b/c", "")
             .relativize("/a/b/c/d/e", "d/e")
-            .relativize("/a/x", "../../x");
+            .relativize("/a/x", "../../x")
+            .relativize("/x", "../../../x");
+        test("a/b/c")
+            .relativize("a/b/c/d", "d")
+            .relativize("a/x", "../../x")
+            .relativize("x", "../../../x")
+            .relativize("", "../../..");
+        test("")
+            .relativize("a", "a")
+            .relativize("a/b/c", "a/b/c")
+            .relativize("", "");
+        test("/")
+            .relativize("/a", "a")
+            .relativize("/a/c", "a/c");
 
         // normalize
         test("/")