Skip to content

Commit

Permalink
[performance] IFile.write(byte[]): reduce store.fetchInfo()
Browse files Browse the repository at this point in the history
Optimistically assume the parent folder already exists on local File
System when it exists in workspace (normal case). So do not explicitly
check it. If it did not exist file write will fail and can be retried
after creating the parent in File System.
This optimization is only implemented for byte[] content in the not
appending case:
 * InputStream content would need a reset.
 * In append mode it is not obvious if something was already appended.

eclipse-platform#1443
  • Loading branch information
EcljpseB0T committed Jun 25, 2024
1 parent 906371d commit 8acce8c
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1211,7 +1211,7 @@ public void write(IFile target, InputStream content, IFileInfo fileInfo, int upd
try (content) {
Resource targetResource = (Resource) target;
IFileStore store = getStore(target);
prepareWrite(target, fileInfo, updateFlags, append, targetResource, store);
prepareWrite(target, fileInfo, updateFlags, append, targetResource, store, false);

// On Windows an attempt to open an output stream on a hidden file results in FileNotFoundException.
// See https://bugs.eclipse.org/bugs/show_bug.cgi?id=194216
Expand Down Expand Up @@ -1243,7 +1243,7 @@ public void write(IFile target, InputStream content, IFileInfo fileInfo, int upd
}

private void prepareWrite(IFile target, IFileInfo fileInfo, int updateFlags, boolean append,
Resource targetResource, IFileStore store) throws CoreException {
Resource targetResource, IFileStore store, boolean optimistic) throws CoreException {
if (fileInfo.getAttribute(EFS.ATTRIBUTE_READ_ONLY)) {
String message = NLS.bind(Messages.localstore_couldNotWriteReadOnly, target.getFullPath());
throw new ResourceException(IResourceStatus.FAILED_WRITE_LOCAL, target.getFullPath(), message, null);
Expand Down Expand Up @@ -1292,11 +1292,14 @@ private void prepareWrite(IFile target, IFileInfo fileInfo, int updateFlags, boo
// never move to the history store, because then the file is missing if write
// fails
getHistoryStore().addState(target.getFullPath(), store, fileInfo, false);
if (!fileInfo.exists()) {
IFileStore parent = store.getParent();
IFileInfo parentInfo = parent.fetchInfo();
if (!parentInfo.exists()) {
parent.mkdir(EFS.NONE, null);
// let's be optimistic that the parent directory exists?
if (!optimistic) {
if (!fileInfo.exists()) {
IFileStore parent = store.getParent();
IFileInfo parentInfo = parent.fetchInfo();
if (!parentInfo.exists()) {
parent.mkdir(EFS.NONE, null);
}
}
}
}
Expand Down Expand Up @@ -1327,7 +1330,7 @@ public void write(IFile target, byte[] content, IFileInfo fileInfo, int updateFl
SubMonitor subMonitor = SubMonitor.convert(monitor, 4);
Resource targetResource = (Resource) target;
IFileStore store = getStore(target);
prepareWrite(target, fileInfo, updateFlags, append, targetResource, store);
prepareWrite(target, fileInfo, updateFlags, append, targetResource, store, true);

// On Windows an attempt to open an output stream on a hidden file results in
// FileNotFoundException.
Expand All @@ -1342,7 +1345,23 @@ public void write(IFile target, byte[] content, IFileInfo fileInfo, int updateFl
subMonitor.split(1);
}
int options = append ? EFS.APPEND : EFS.NONE;
store.write(content, options, subMonitor.split(1));
try {
store.write(content, options, subMonitor.split(1));
} catch (CoreException e) {
if (append) {
throw e;
}
if (e.getStatus().getCode() != EFS.ERROR_WRITE) {
throw e;
}
IFileStore parent = store.getParent();
IFileInfo parentInfo = parent.fetchInfo();
if (!parentInfo.exists()) {
parent.mkdir(EFS.NONE, null);
// try again
store.write(content, options, null);
}
}
if (restoreHiddenAttribute) {
fileInfo.setAttribute(EFS.ATTRIBUTE_HIDDEN, true);
store.putInfo(fileInfo, EFS.SET_ATTRIBUTES, subMonitor.split(1));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.atomic.AtomicInteger;
import org.eclipse.core.internal.resources.ResourceException;
import org.eclipse.core.resources.IContainer;
import org.eclipse.core.resources.IFile;
import org.eclipse.core.resources.IFileState;
Expand Down Expand Up @@ -484,6 +485,27 @@ public void testCreateBytes() throws CoreException {
assertFalse(derived.isDerived());
assertFalse(derived.isTeamPrivateMember());
assertEquals("notDerived", derived.readString());

IFolder subFolder = projects[0].getFolder("subFolder");
subFolder.create(true, true, null);
subFolder.getRawLocation().toFile().delete();

IFile orphan = subFolder.getFile("myParentDoesNotExist.txt");
monitor.prepare();
orphan.write("parentDoesNotExistInFileSystemButInWorkspace".getBytes(), true, false, false, monitor);
monitor.assertUsedUp();
assertEquals("parentDoesNotExistInFileSystemButInWorkspace", orphan.readString());

monitor.prepare();
orphan.getParent().delete(true, null);
// if the parent is deleted in workspace Exception is expected:
try {
orphan.write("parentDoesNotExist - not even in workspace".getBytes(), true, false, false, monitor);
assertFalse("should not be reached", true);
} catch (ResourceException expected) {
monitor.assertUsedUp();
assertFalse(orphan.exists());
}
}

@Test
Expand Down

0 comments on commit 8acce8c

Please sign in to comment.