Skip to content

Commit

Permalink
vfs: do not traverse all elements when building directory listing
Browse files Browse the repository at this point in the history
Motivation:
DirectoryStream accepts NavigableSet for directory listing to allow
file system implementations to optimize the ordering and populate the
listing in lazy fashion is required. However, PseudoFs#list method
applies transformation and convert it into a collection, which enforces
full iteration over provided listing. Moreover, the newly created
collection is converted into TreeSet and re-sorted.

Modification:
Introduce DirectoryStream#transform method which returns a new
DirectoryStream with lazy-transformed elements. The getEntries() method
is removed.

Result:
faster directory listing on big directories with less memory
consumption. On a directory with 100K files the ls operation
is ~x10 faster!

Fixes: #61
Acked-by: Dmitry Litvintsev
Target: master, 0.17
  • Loading branch information
kofemann committed Oct 5, 2018
1 parent 8989a69 commit 7c93dba
Show file tree
Hide file tree
Showing 3 changed files with 167 additions and 17 deletions.
81 changes: 68 additions & 13 deletions core/src/main/java/org/dcache/nfs/vfs/DirectoryStream.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2009 - 2017 Deutsches Elektronen-Synchroton,
* Copyright (c) 2009 - 2018 Deutsches Elektronen-Synchroton,
* Member of the Helmholtz Association, (DESY), HAMBURG, GERMANY
*
* This library is free software; you can redistribute it and/or modify
Expand All @@ -19,12 +19,13 @@
*/
package org.dcache.nfs.vfs;

import com.google.common.collect.Sets;
import com.google.common.collect.ForwardingNavigableSet;
import java.util.Collection;
import java.util.Collections;
import java.util.Iterator;
import java.util.NavigableSet;
import java.util.SortedSet;
import java.util.TreeSet;
import java.util.function.Function;
import org.dcache.nfs.v4.xdr.nfs4_prot;

/**
Expand All @@ -48,22 +49,13 @@ public DirectoryStream(byte[] verifier, Collection<DirectoryEntry> entries) {

public DirectoryStream(byte[] verifier, NavigableSet<DirectoryEntry> entries) {
this.verifier = verifier;
this.entries = Sets.unmodifiableNavigableSet(entries);
this.entries = Collections.unmodifiableNavigableSet(entries);
}

public byte[] getVerifier() {
return verifier;
}

/**
* Get listing entries sorted by cookies.
*
* @return listing entries.
*/
public SortedSet<DirectoryEntry> getEntries() {
return entries;
}

@Override
public Iterator<DirectoryEntry> iterator() {
return entries.iterator();
Expand All @@ -81,4 +73,67 @@ public DirectoryStream tail(long fromCookie) {
final DirectoryEntry cookieEntry = new DirectoryEntry("", null, null, fromCookie);
return new DirectoryStream(verifier, entries.tailSet(cookieEntry, false));
}

/**
* Returns a {@link DirectoryStream} that applies {@code function} to
* each entry in this stream.
*
* @param function a transformation function to apply.
* @return the new stream with transformed elements.
*/
public DirectoryStream transform(Function<? super DirectoryEntry, DirectoryEntry> function) {
return new DirectoryStream(this.verifier, new TransformingNavigableSet(function, entries));
}

private static class TransformingNavigableSet extends ForwardingNavigableSet<DirectoryEntry> {

private final Function<? super DirectoryEntry, DirectoryEntry> transformation;
private final NavigableSet<DirectoryEntry> inner;

public TransformingNavigableSet(Function<? super DirectoryEntry, DirectoryEntry> transformation, NavigableSet<DirectoryEntry> inner) {
this.transformation = transformation;
this.inner = inner;
}

@Override
protected NavigableSet<DirectoryEntry> delegate() {
return inner;
}

@Override
public Iterator<DirectoryEntry> iterator() {
return new TransformingIterator(transformation, delegate().iterator());
}

/*
* the result of ForwardingNavigableSet#tailSet does not inherit
* overloaded iterator, thus we have to override it again.
*/
@Override
public NavigableSet<DirectoryEntry> tailSet(DirectoryEntry fromElement, boolean inclusive) {
return new TransformingNavigableSet(transformation, super.tailSet(fromElement, inclusive));
}
}

// iterator decorator.
private static class TransformingIterator implements Iterator<DirectoryEntry> {

private final Function<? super DirectoryEntry, DirectoryEntry> transformation;
private final Iterator<DirectoryEntry> inner;

public TransformingIterator(Function<? super DirectoryEntry, DirectoryEntry> transformation, Iterator<DirectoryEntry> inner) {
this.transformation = transformation;
this.inner = inner;
}

@Override
public boolean hasNext() {
return inner.hasNext();
}

@Override
public DirectoryEntry next() {
return transformation.apply(inner.next());
}
}
}
8 changes: 4 additions & 4 deletions core/src/main/java/org/dcache/nfs/vfs/PseudoFs.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,15 @@
*/
package org.dcache.nfs.vfs;

import com.google.common.base.Function;
import com.google.common.base.Splitter;
import com.google.common.collect.Collections2;
import java.io.IOException;
import java.net.InetAddress;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.function.Function;
import javax.security.auth.Subject;
import org.dcache.auth.Subjects;
import org.dcache.nfs.ChimeraNFSException;
Expand All @@ -51,6 +50,7 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import static com.google.common.collect.Lists.newArrayList;
import static org.dcache.nfs.vfs.AclCheckable.Access;
/**
* A decorated {@code VirtualFileSystem} that builds a Pseudo file system
Expand Down Expand Up @@ -209,7 +209,7 @@ public DirectoryStream list(Inode inode, byte[] verifier, long cookie) throws IO
return new DirectoryStream(listPseudoDirectory(inode));
}
DirectoryStream innerStrem = _inner.list(inode, verifier, cookie);
return new DirectoryStream(innerStrem.getVerifier(), Collections2.transform(innerStrem.getEntries(), new PushParentIndex(inode)));
return innerStrem.transform(new PushParentIndex(inode));
}

@Override
Expand Down Expand Up @@ -513,7 +513,7 @@ private Collection<DirectoryEntry> listPseudoDirectory(Inode parent) throws Chim
for (PseudoFsNode node : nodes) {
if (node.id().equals(parent)) {
if (node.isMountPoint()) {
return Collections2.transform(_inner.list(parent, null, 0L).getEntries(), new ConvertToRealInode(node));
return newArrayList(_inner.list(parent, null, 0L).transform(new ConvertToRealInode(node)));
} else {
long cookie = 0; // artificial cookie
List<DirectoryEntry> pseudoLs = new ArrayList<>();
Expand Down
95 changes: 95 additions & 0 deletions core/src/test/java/org/dcache/nfs/vfs/DirectoryStreamTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
package org.dcache.nfs.vfs;

import com.google.common.primitives.Ints;
import java.util.NavigableSet;
import java.util.TreeSet;
import org.dcache.nfs.v4.xdr.verifier4;
import org.junit.Test;
import static org.junit.Assert.*;
import org.junit.Before;

/**
*
*/
public class DirectoryStreamTest {

private DirectoryStream stream;
@Before
public void setUp() {

NavigableSet<DirectoryEntry> entries = new TreeSet<>();
byte[] verifier = verifier4.valueOf(System.currentTimeMillis()).value;

for (int i = 0; i < 10; i++) {
FileHandle fh = new FileHandle(0, 0, 0, Ints.toByteArray(i));
Inode inode = new Inode(fh);
Stat stat = new Stat();

stat.setMode(Stat.S_IFDIR | 0755);
stat.setMTime(System.currentTimeMillis());
stat.setATime(System.currentTimeMillis());
stat.setCTime(System.currentTimeMillis());
stat.setGeneration(1);
stat.setNlink(2);
stat.setUid(1);
stat.setGid(2);
stat.setDev(1);
stat.setFileid(i);
stat.setSize(512);

entries.add(new DirectoryEntry(String.format("file-%d", i), inode, stat, i));
}

stream = new DirectoryStream(verifier, entries);
}

@Test
public void testTail() {
DirectoryStream tail = stream.tail(3);
assertEquals(tail.iterator().next().getCookie(), 4);
assertArrayEquals(stream.getVerifier(), tail.getVerifier());
}

@Test
public void testTransform() {
DirectoryStream transformed = stream.transform(d -> new DirectoryEntry(d.getName().toUpperCase(), d.getInode(), d.getStat(), d.getCookie()));
assertArrayEquals(stream.getVerifier(), transformed.getVerifier());
for (DirectoryEntry e : transformed) {
assertTrue(e.getName().startsWith("FILE"));
}
}

@Test
public void testTransformAndTail() {

DirectoryStream transformed = stream.transform(d -> new DirectoryEntry(d.getName().toUpperCase(), d.getInode(), d.getStat(), d.getCookie()));
DirectoryStream tail = transformed.tail(3);
DirectoryEntry next = tail.iterator().next();

assertEquals(next.getCookie(), 4);
assertEquals("FILE-4", next.getName());

}

@Test
public void testMultipleTransformAndTail() {
DirectoryStream transformed = stream.transform(d -> new DirectoryEntry(d.getName().toUpperCase(), d.getInode(), d.getStat(), d.getCookie()));
DirectoryStream tail = transformed.tail(3).tail(5);
DirectoryEntry next = tail.iterator().next();

assertEquals(next.getCookie(), 6);
assertEquals("FILE-6", next.getName());
}

@Test
public void testMultipleTransform() {
DirectoryStream transformed = stream
.transform(d -> new DirectoryEntry(d.getName().toUpperCase(), d.getInode(), d.getStat(), d.getCookie()))
.transform(d -> new DirectoryEntry("a" + d.getName(), d.getInode(), d.getStat(), d.getCookie()));

assertArrayEquals(stream.getVerifier(), transformed.getVerifier());
for (DirectoryEntry e : transformed) {
assertTrue(e.getName().startsWith("aFILE"));
}
}
}

0 comments on commit 7c93dba

Please sign in to comment.