Skip to content

Commit

Permalink
Move to Files.new* creation of I/O streams during IDE boot
Browse files Browse the repository at this point in the history 
FileOutputStream and FileInputStream complicate GC because they both
override `#finalize()`. Switching to NIO API creates Stream without
finalizers and thus relieving GC.
  • Loading branch information
@mdindoffer @matthiasblaesing
mdindoffer authored and matthiasblaesing committed Jan 14, 2018
1 parent dfcc859 commit 4b82e6a
Show file tree
Hide file tree
Showing 20 changed files with 165 additions and 198 deletions.
2 changes: 1 addition & 1 deletion core.netigso/nbproject/project.properties
View file
Expand Up @@ -16,7 +16,7 @@
# under the License.

is.autoload=true
javac.source=1.6
javac.source=1.7
javac.compilerargs=-Xlint -Xlint:-serial
javadoc.arch=${basedir}/arch.xml
javadoc.apichanges=${basedir}/apichanges.xml
Expand Down
12 changes: 7 additions & 5 deletions core.netigso/src/org/netbeans/core/netigso/Netigso.java
View file
Expand Up @@ -22,10 +22,11 @@
import java.io.ByteArrayOutputStream;
import java.io.DataOutputStream;
import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.net.URL;
import java.nio.file.Files;
import java.nio.file.InvalidPathException;
import java.security.ProtectionDomain;
import java.util.Arrays;
import java.util.Collection;
Expand All @@ -36,7 +37,6 @@
import java.util.Map;
import java.util.Properties;
import java.util.Set;
import java.util.StringTokenizer;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.jar.JarEntry;
import java.util.jar.JarFile;
Expand Down Expand Up @@ -453,9 +453,11 @@ private void fakeOneModule(Module m, Bundle original) throws IOException {
} else if (symbolicName != null) { // NOI18N
if (original != null) {
LOG.log(Level.FINE, "Updating bundle {0}", original.getLocation());
FileInputStream is = new FileInputStream(m.getJarFile());
original.update(is);
is.close();
try (InputStream is = Files.newInputStream(m.getJarFile().toPath())) {
original.update(is);
} catch (InvalidPathException ex) {
throw new IOException(ex);
}
b = original;
} else {
BundleContext bc = framework.getBundleContext();
Expand Down
10 changes: 5 additions & 5 deletions core.network/src/org/netbeans/core/network/proxy/NbProxySelector.java
View file
Expand Up @@ -21,10 +21,11 @@

import java.io.BufferedInputStream;
import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.net.*;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.util.*;
import java.util.logging.Level;
import java.util.logging.Logger;
Expand Down Expand Up @@ -421,10 +422,9 @@ static boolean useSystemProxies() {
}

fname = netProperties.getCanonicalPath();
InputStream in = new FileInputStream(fname);
BufferedInputStream bin = new BufferedInputStream(in);
props.load(bin);
bin.close();
try (InputStream bin = new BufferedInputStream(Files.newInputStream(Paths.get(fname)))) {
props.load(bin);
}

String val = props.getProperty(propertyKey);
val = System.getProperty(propertyKey, val);
Expand Down
17 changes: 5 additions & 12 deletions core.network/src/org/netbeans/core/network/proxy/kde/KdeNetworkProxy.java
View file
Expand Up @@ -19,12 +19,11 @@
package org.netbeans.core.network.proxy.kde;

import java.io.BufferedReader;
import java.io.DataInputStream;
import java.io.File;
import java.io.FileInputStream;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.InputStreamReader;
import java.nio.file.Files;
import java.nio.file.InvalidPathException;
import java.util.HashMap;
import java.util.Map;
import java.util.logging.Level;
Expand Down Expand Up @@ -133,10 +132,7 @@ private Map<String, String> getKioslavercMap() {
Map<String, String> map = new HashMap<String, String>();

if (kioslavercFile.exists()) {
try {
FileInputStream fis = new FileInputStream(kioslavercFile);
DataInputStream dis = new DataInputStream(fis);
BufferedReader br = new BufferedReader(new InputStreamReader(dis));
try (BufferedReader br = Files.newBufferedReader(kioslavercFile.toPath())) {
String line;
boolean inGroup = false;
while ((line = br.readLine()) != null) {
Expand All @@ -153,11 +149,8 @@ private Map<String, String> getKioslavercMap() {
inGroup = true;
}
}
dis.close();
} catch (FileNotFoundException fnfe) {
LOGGER.log(Level.SEVERE, "Cannot read file: ", fnfe);
} catch (IOException ioe) {
LOGGER.log(Level.SEVERE, "Cannot read file: ", ioe);
} catch (IOException | InvalidPathException ex) {
LOGGER.log(Level.SEVERE, "Cannot read file: ", ex);
}
} else {
LOGGER.log(Level.WARNING, "KDE system proxy resolver: The kioslaverc file not found ({0})", KIOSLAVERC_PATH);
Expand Down
2 changes: 1 addition & 1 deletion core.osgi/nbproject/project.properties
View file
Expand Up @@ -15,7 +15,7 @@
# specific language governing permissions and limitations
# under the License.
is.autoload=true
javac.source=1.6
javac.source=1.7
javac.compilerargs=-Xlint -Xlint:-serial
cp.extra=\
${nb_all}/libs.osgi/external/osgi.core-5.0.0.jar:\
Expand Down
24 changes: 9 additions & 15 deletions core.osgi/src/org/netbeans/core/osgi/OSGiInstalledFileLocator.java
View file
Expand Up @@ -20,12 +20,13 @@
package org.netbeans.core.osgi;

import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.net.URI;
import java.net.URL;
import java.nio.file.Files;
import java.nio.file.InvalidPathException;
import java.util.Arrays;
import java.util.Collections;
import java.util.Iterator;
Expand Down Expand Up @@ -122,27 +123,20 @@ public OSGiInstalledFileLocator(BundleContext context) {
if (!dir.isDirectory() && !dir.mkdirs()) {
throw new IOException("Could not make " + dir);
}
InputStream is = resource.openStream();
try {
OutputStream os = new FileOutputStream(f2);
try {
byte[] buf = new byte[4096];
int read;
while ((read = is.read(buf)) != -1) {
os.write(buf, 0, read);
}
} finally {
os.close();
try (InputStream is = resource.openStream();
OutputStream os = Files.newOutputStream(f2.toPath())) {
byte[] buf = new byte[4096];
int read;
while ((read = is.read(buf)) != -1) {
os.write(buf, 0, read);
}
} finally {
is.close();
}
if (execFiles.contains(name)) {
f2.setExecutable(true);
}
}
return f;
} catch (IOException x) {
} catch (IOException | InvalidPathException x) {
Exceptions.printStackTrace(x);
}
}
Expand Down
2 changes: 1 addition & 1 deletion core.output2/nbproject/project.properties
View file
Expand Up @@ -17,7 +17,7 @@

is.autoload=true
javac.compilerargs=-Xlint -Xlint:-serial
javac.source=1.6
javac.source=1.7
javadoc.arch=${basedir}/arch.xml

test.config.stableBTD.includes=**/*Test.class
Expand Down
45 changes: 23 additions & 22 deletions core.output2/src/org/netbeans/core/output2/AbstractLines.java
View file
Expand Up @@ -22,13 +22,15 @@
import java.awt.Color;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.io.FileOutputStream;
import java.io.IOException;
import java.nio.ByteBuffer;
import java.nio.CharBuffer;
import java.nio.channels.FileChannel;
import java.nio.charset.Charset;
import java.nio.charset.CharsetEncoder;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.StandardOpenOption;
import java.util.Collection;
import java.util.Collections;
import java.util.concurrent.atomic.AtomicBoolean;
Expand Down Expand Up @@ -869,7 +871,6 @@ public void saveAs(String path) throws IOException {
if (storage == null) {
throw new IOException ("Data has already been disposed"); //NOI18N
}
FileOutputStream fos = new FileOutputStream(path);
try {
String encoding = System.getProperty ("file.encoding"); //NOI18N
if (encoding == null) {
Expand All @@ -878,30 +879,30 @@ public void saveAs(String path) throws IOException {
Charset charset = Charset.forName (encoding); //NOI18N
CharsetEncoder encoder = charset.newEncoder ();
String ls = System.getProperty("line.separator");
FileChannel ch = fos.getChannel();
ByteBuffer lsbb = encoder.encode(CharBuffer.wrap(ls));
for (int i = 0; i < getLineCount(); i++) {
int lineStart = getCharLineStart(i);
int lineLength = length(i);
BufferResource<CharBuffer> br = getCharBuffer(lineStart,
lineLength);
try {
CharBuffer cb = br.getBuffer();
ByteBuffer bb = encoder.encode(cb);
ch.write(bb);
if (i != getLineCount() - 1) {
lsbb.rewind();
ch.write(lsbb);
}
} finally {
if (br != null) {
br.releaseBuffer();
Path filePath = Paths.get(path);
try (FileChannel ch = FileChannel.open(filePath, StandardOpenOption.WRITE, StandardOpenOption.CREATE)) {
ByteBuffer lsbb = encoder.encode(CharBuffer.wrap(ls));
for (int i = 0; i < getLineCount(); i++) {
int lineStart = getCharLineStart(i);
int lineLength = length(i);
BufferResource<CharBuffer> br = getCharBuffer(lineStart,
lineLength);
try {
CharBuffer cb = br.getBuffer();
ByteBuffer bb = encoder.encode(cb);
ch.write(bb);
if (i != getLineCount() - 1) {
lsbb.rewind();
ch.write(lsbb);
}
} finally {
if (br != null) {
br.releaseBuffer();
}
}
}
}
ch.close();
} finally {
fos.close();
FileUtil.refreshFor(new java.io.File(path));
}
}
Expand Down
4 changes: 2 additions & 2 deletions core.output2/src/org/netbeans/core/output2/Controller.java
View file
Expand Up @@ -23,10 +23,10 @@
import java.awt.Font;
import java.io.CharConversionException;
import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.OutputStream;
import java.lang.ref.WeakReference;
import java.nio.file.Files;
import java.util.HashSet;
import java.util.LinkedList;
import java.util.List;
Expand Down Expand Up @@ -525,7 +525,7 @@ private static OutputStream getLogStream() {
f.delete();
}
f.createNewFile();
logStream = new FileOutputStream(f);
logStream = Files.newOutputStream(f.toPath());
} catch (Exception e) {
e.printStackTrace();
logStream = System.err;
Expand Down
2 changes: 1 addition & 1 deletion core.startup.base/nbproject/project.properties
View file
Expand Up @@ -14,7 +14,7 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
javac.source=1.6
javac.source=1.7
javac.compilerargs=-Xlint -Xlint:-serial
spec.version.base=1.63.0
module.jar.dir=core
Expand Down
17 changes: 6 additions & 11 deletions core.startup.base/src/org/netbeans/core/startup/layers/ArchiveURLMapper.java
View file
Expand Up @@ -20,7 +20,6 @@
package org.netbeans.core.startup.layers;

import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
Expand All @@ -30,6 +29,8 @@
import java.net.URISyntaxException;
import java.net.URL;
import java.net.URLDecoder;
import java.nio.file.Files;
import java.nio.file.InvalidPathException;
import java.util.Enumeration;
import java.util.HashMap;
import java.util.Map;
Expand Down Expand Up @@ -241,16 +242,10 @@ private static File copyJAR(FileObject fo, URI archiveFileURI, boolean replace)
copy = copy.getCanonicalFile();
copy.deleteOnExit();
}
InputStream is = fo.getInputStream();
try {
OutputStream os = new FileOutputStream(copy);
try {
FileUtil.copy(is, os);
} finally {
os.close();
}
} finally {
is.close();
try (InputStream is = fo.getInputStream(); OutputStream os = Files.newOutputStream(copy.toPath())) {
FileUtil.copy(is, os);
} catch (InvalidPathException ex) {
throw new IOException(ex);
}
copiedJARs.put(archiveFileURI, copy);
}
Expand Down
9 changes: 7 additions & 2 deletions core.startup.base/src/org/netbeans/core/startup/layers/NbinstURLStreamHandler.java
View file
Expand Up @@ -20,7 +20,6 @@
package org.netbeans.core.startup.layers;

import java.io.File;
import java.io.FileInputStream;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.InputStream;
Expand All @@ -29,6 +28,8 @@
import java.net.URLConnection;
import java.net.URLStreamHandler;
import java.net.UnknownServiceException;
import java.nio.file.Files;
import java.nio.file.InvalidPathException;
import org.openide.filesystems.FileObject;
import org.openide.filesystems.FileUtil;
import org.openide.util.Exceptions;
Expand Down Expand Up @@ -140,7 +141,11 @@ public int getContentLength() {
public InputStream getInputStream() throws IOException {
this.connect();
if (iStream == null) {
iStream = new FileInputStream(f);
try {
iStream = Files.newInputStream(f.toPath());
} catch (InvalidPathException ex) {
throw new IOException(ex);
}
}
return iStream;
}
Expand Down
16 changes: 8 additions & 8 deletions core.startup.base/test/unit/src/org/netbeans/core/startup/layers/SystemFileSystemTest.java
View file
Expand Up @@ -34,7 +34,6 @@
import org.netbeans.core.startup.Main;
import org.netbeans.core.startup.MainLookup;
import org.netbeans.SetupHid;
import org.netbeans.core.startup.layers.SystemFileSystem;
import org.openide.filesystems.FileAttributeEvent;
import org.openide.filesystems.FileChangeListener;
import org.openide.filesystems.FileEvent;
Expand Down Expand Up @@ -236,17 +235,18 @@ public void testCanHideFilesFromModules() throws Exception {
}

private static void write(FileObject fo, String txt) throws IOException {
OutputStream os = fo.getOutputStream();
os.write(txt.getBytes());
os.close();
try (OutputStream os = fo.getOutputStream()) {
os.write(txt.getBytes());
}
}

private static String read(FileObject fo) throws IOException {
byte[] arr = new byte[(int)fo.getSize()];
InputStream is = fo.getInputStream();
int len = is.read(arr);
assertEquals("Not enough read", arr.length, len);
return new String(arr);
try (InputStream is = fo.getInputStream()) {
int len = is.read(arr);
assertEquals("Not enough read", arr.length, len);
return new String(arr);
}
}

public FileSystem convert(FileSystem obj) {
Expand Down

2 comments on commit 4b82e6a

@eirikbakke
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this bug might be related to this: https://issues.apache.org/jira/browse/NETBEANS-583
(Switching to NIO means ClosedByInterruptException can now be thrown in various places.)

@eirikbakke
Copy link
Contributor

@eirikbakke eirikbakke commented on 4b82e6a Sep 19, 2018 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking over a few of the places touched in this commit, it seems a more cautious approach is needed; the ClosedByInterruptException must always be handled somehow. The options are:

  1. For methods called by clients, throw an InterruptedException. That would require API changes, and so is not an option.
  2. For methods called by clients, where an IOException is already specified in the API, just propagate the ClosedByInterruptException. That makes what was previously not an error condition (a thread being interrupted), into an error condition. So this is not an option.
  3. For methods where we control all the call sites, handle the ClosedByInterruptException specially by stopping execution in a controlled manner and returning a default result or similar, and setting the current thread's interrupt bit. The problem with this is that it's hard to ascertain that the incorrect "default" result isn't being cached somewhere, especially in higher-level client code. (E.g. if the loading of a proxy configuration file is interrupted and we just return some default settings.)
  4. Handle ClosedByInterruptException by automatically retrying the operation in question, perhaps with the old non-interruptible streams, until the operation succeeds (or throws a non-interrupt-related IOException). Then set the current thread's interrupt bit. This retains the old behavior.

I think (4) is the best option here. But it would need to be implemented as custom "non-interruptible" FileInputStream/FileOutputStream that wraps the NIO ones, as in some cases the stream objects are exposed via public APIs. Even when stream objects are not exposed via public APIs, changing every place that uses a NIO FileInputStream/FileInputStream to incorporate the retry logic, as was done in #854 for JarClassLoader, would clutter up a lot of code unnecessarily.

A final option is to just revert to the old FileInputStream/FileOutputStream classes. Was there a significant performance benefit to avoiding them?

Please sign in to comment.