Skip to content

Improve startup cache loading performance #9307

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mbien wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Improve startup cache loading performance #9307

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
140 changes: 88 additions & 52 deletions platform/core.startup/src/org/netbeans/core/startup/ModuleList.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,12 @@
import java.io.BufferedInputStream;
import java.io.ByteArrayInputStream;
import java.io.CharArrayWriter;
import java.io.DataInputStream;
import java.io.DataOutputStream;
import java.io.File;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.InputStream;
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
import java.io.OutputStream;
import java.io.OutputStreamWriter;
import java.io.PushbackInputStream;
Expand All @@ -51,6 +50,7 @@
import java.util.jar.JarFile;
import java.util.logging.Level;
import java.util.logging.Logger;
import java.util.stream.Collectors;
import org.netbeans.DuplicateException;
import org.netbeans.Events;
import org.netbeans.InvalidException;
Expand Down Expand Up @@ -111,7 +111,7 @@ final class ModuleList implements Stamps.Updater {
/** to fire events with */
private final Events ev;
/** map from code name (base)s to statuses of modules on disk */
private final Map<String,DiskStatus> statuses = new HashMap<String,DiskStatus>(100);
private final Map<String, DiskStatus> statuses = new HashMap<>(100);
/** whether the initial round has been triggered or not */
private boolean triggered = false;
/** listener for changes in modules, etc.; see comment on class Listener */
Expand Down Expand Up @@ -197,8 +197,7 @@ private File findJarByName(String jar, String name) throws IOException {
for (File candidate : jars) {
int candidateMajor = -1;
SpecificationVersion candidateSpec = null;
JarFile jf = new JarFile(candidate);
try {
try (JarFile jf = new JarFile(candidate)) {
java.util.jar.Attributes attr = jf.getManifest().getMainAttributes();
String codename = attr.getValue("OpenIDE-Module");
if (codename != null) {
Expand All @@ -211,8 +210,6 @@ private File findJarByName(String jar, String name) throws IOException {
if (sv != null) {
candidateSpec = new SpecificationVersion(sv);
}
} finally {
jf.close();
}
if (newest == null || candidateMajor > major || (spec != null && candidateSpec != null && candidateSpec.compareTo(spec) > 0)) {
newest = candidate;
Expand Down Expand Up @@ -434,7 +431,10 @@ private Object processStatusParam(String k, String v) throws NumberFormatExcepti

/** Just checks that all the right stuff is there.
*/
private void sanityCheckStatus(Map<String,Object> m) throws IOException {
private static void sanityCheckStatus(Map<String, Object> m) throws IOException {
if (m.isEmpty()) {
throw new IOException("Must define properties"); // NOI18N
}
String jar = (String) m.get("jar"); // NOI18N
if (jar == null) {
throw new IOException("Must define jar param"); // NOI18N
Expand Down Expand Up @@ -612,45 +612,64 @@ private String readTo(InputStream is, char delim) throws IOException {
}
}

final Map<String,Map<String,Object>> readCache() {
final Map<String, Map<String, Object>> readCache() {
InputStream is = Stamps.getModulesJARs().asStream("all-modules.dat"); // NOI18N
if (is == null) {
// schedule write for later
writeCache();
return null;
}
LOG.log(Level.FINEST, "Reading cache all-modules.dat");
try {
ObjectInputStream ois = new ObjectInputStream(is);

Map<String,Map<String,Object>> ret = new HashMap<String, Map<String, Object>>(1333);
while (is.available() > 0) {
Map<String, Object> prop = readStatus(ois, false);
if (prop == null) {
LOG.log(Level.CONFIG, "Cache is invalid all-modules.dat");
return null;
}
Set<?> deps;
try {
deps = (Set<?>) ois.readObject();
} catch (ClassNotFoundException ex) {
throw new IOException(ex);
}
prop.put("deps", deps);
String cnb = (String)prop.get("name"); // NOI18N
ret.put(cnb, prop);
try (DataInputStream dis = new DataInputStream(is)) {
Map<String, Map<String, Object>> cache = new HashMap<>(1333);
while (dis.available() > 0) {
Map<String, Object> props = readProps(dis);
props.put("deps", readDeps(dis));
cache.put((String) props.get("name"), props);
}


is.close();
return ret;
return cache;
} catch (IOException ex) {
LOG.log(Level.INFO, "Cannot read cache", ex);
writeCache();
return null;
}
}

private static Set<Dependency> readDeps(DataInputStream is) throws IOException {
int depCount = is.readInt();
if (depCount < 0) {
throw new IOException("negative count");
} else if (depCount == 0) {
return Set.of();
}
Set<Dependency> deps = new HashSet<>((int) Math.ceil(depCount / 0.75));
for (int i = 0; i < depCount; i++) {
deps.add(Dependency.read(is));
}
return deps;
}

private static Map<String, Object> readProps(DataInputStream is) throws IOException {
Map<String, Object> props = new HashMap<>(8);
for (String str : is.readUTF().split(",")) {
String[] entry = str.split("=", 2);
// must match computeProperties()
try {
Object val = switch (entry[0]) {
case "name", "jar" -> entry[1];
case "enabled", "autoload", "eager", "reloadable" -> Boolean.valueOf(entry[1]);
case "startlevel" -> Integer.valueOf(entry[1]);
default -> throw new IOException("unknown key " + entry[0]);
};
props.put(entry[0], val);
} catch (IllegalArgumentException | ArrayIndexOutOfBoundsException ex) {
throw new IOException("unexpected value", ex);
}
}
sanityCheckStatus(props);
return props;
}

final void writeCache() {
Stamps.getModulesJARs().scheduleSave(this, "all-modules.dat", false);
}
Expand All @@ -661,14 +680,23 @@ public void cacheReady() {

@Override
public void flushCaches(DataOutputStream os) throws IOException {
ObjectOutputStream oss = new ObjectOutputStream(os);
for (Module m : mgr.getModules()) {
if (m.isFixed()) {
continue;
}
Map<String, Object> prop = computeProperties(m);
writeStatus(prop, oss);
oss.writeObject(m.getDependencies());
// props
String list = computeProperties(m).entrySet()
.stream()
.map(e -> e.getKey() + "=" + e.getValue())
.collect(Collectors.joining(","));
Comment on lines +690 to +691
Copy link
Copy Markdown
Contributor

@matthiasblaesing matthiasblaesing Apr 3, 2026

Choose a reason for hiding this comment

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

This will break one either a key or a value contains = or ,.

Copy link
Copy Markdown
Member Author

@mbien mbien Apr 3, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

yes but can this happen? The set of keys is restricted to (see readProps):

                    case "name", "jar" -> entry[1];
                    case "enabled", "autoload", "eager", "reloadable" -> Boolean.valueOf(entry[1]);
                    case "startlevel" -> Integer.valueOf(entry[1]);
                    default -> throw new IOException("unknown key " + entry[0]);

the first two are String values, the rest are primitives.

This does not cache all properties, only a very limited set. (this was already done before this change)

Copy link
Copy Markdown
Contributor

@matthiasblaesing matthiasblaesing Apr 6, 2026

Choose a reason for hiding this comment

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

The problem with these ad-hoc encoding is, that they have the tendency to creep up later and hunt you when you least expect it.

name currently is the codename base, at this point in time to my knowledge CNDs can't contain both, but that is not set into stone.

Not sure that a naming convention for jar files is enforced for modules, but at its core it is a filename and that can hold both , and =.

Why not reuse an approach you are already using: don't invent a new serialization, but write the number of key/value pairs to the stream and then the keys and values as pure strings. That way there is no need to consider "special" characters.

os.writeUTF(list);

// deps
Set<Dependency> deps = m.getDependencies();
os.writeInt(deps.size());
for (Dependency dep : deps) {
dep.write(os);
}
}
}

Expand All @@ -694,7 +722,7 @@ private void writeStatus(Map<String, Object> m, OutputStream os) throws IOExcept

// Use TreeMap to sort the keys by name; since the module status files might
// be version-controlled we want to avoid gratuitous format changes.
for (Map.Entry<String, Object> entry: new TreeMap<String, Object>(m).entrySet()) {
for (Map.Entry<String, Object> entry : new TreeMap<>(m).entrySet()) {
String name = entry.getKey();
if (
name.equals("name") || // NOI18N
Expand Down Expand Up @@ -749,11 +777,8 @@ public void run() throws IOException {
LOG.fine("ModuleList: (re)writing " + nue.file);
FileLock lock = nue.file.lock();
try {
OutputStream os = nue.file.getOutputStream(lock);
try {
try (OutputStream os = nue.file.getOutputStream(lock)) {
writeStatus(nue.diskProps, os);
} finally {
os.close();
}
} finally {
lock.releaseLock();
Expand Down Expand Up @@ -944,12 +969,13 @@ private void moduleChanged(Module m, DiskStatus status) {
}
}

/** Compute what properties we would want to store in XML
/**
* Compute what properties we would want to store in XML (or the startup cache)
* for this module. I.e. 'name', 'reloadable', etc.
*/
private Map<String,Object> computeProperties(Module m) {
private Map<String, Object> computeProperties(Module m) {
if (m.isFixed() || ! m.isValid()) throw new IllegalArgumentException("fixed or invalid: " + m); // NOI18N
Map<String,Object> p = new HashMap<String,Object>();
Map<String, Object> p = new HashMap<>(8);
p.put("name", m.getCodeNameBase()); // NOI18N
if (!m.isAutoload() && !m.isEager()) {
p.put("enabled", m.isEnabled()); // NOI18N
Expand All @@ -960,8 +986,7 @@ private Map<String,Object> computeProperties(Module m) {
if (m.getStartLevel() > 0) {
p.put("startlevel", m.getStartLevel()); // NOI18N
}
if (m.getHistory() instanceof ModuleHistory) {
ModuleHistory hist = (ModuleHistory) m.getHistory();
if (m.getHistory() instanceof ModuleHistory hist) {
p.put("jar", hist.getJar()); // NOI18N
}
return p;
Expand Down Expand Up @@ -990,6 +1015,7 @@ private final class Listener implements PropertyChangeListener, ErrorHandler, En
// Property change coming from ModuleManager or some known Module.

private boolean listening = true;
@Override
public void propertyChange(PropertyChangeEvent evt) {
if (! triggered) throw new IllegalStateException("Property change before trigger()"); // NOI18N
// REMEMBER this is inside *read* mutex, it is forbidden to even attempt
Expand Down Expand Up @@ -1036,15 +1062,19 @@ public void propertyChange(PropertyChangeEvent evt) {

// SAX stuff.

@Override
public void warning(SAXParseException e) throws SAXException {
LOG.log(Level.WARNING, null, e);
}
@Override
public void error(SAXParseException e) throws SAXException {
throw e;
}
@Override
public void fatalError(SAXParseException e) throws SAXException {
throw e;
}
@Override
public InputSource resolveEntity(String pubid, String sysid) throws SAXException, IOException {
if (pubid.equals(PUBLIC_ID)) {
if (VALIDATE_XML) {
Expand All @@ -1062,6 +1092,7 @@ public InputSource resolveEntity(String pubid, String sysid) throws SAXException

// Changes in Modules/ folder.

@Override
public void fileDeleted(FileEvent ev) {
if (isOurs(ev)) {
if (LOG.isLoggable(Level.FINE)) {
Expand All @@ -1073,6 +1104,7 @@ public void fileDeleted(FileEvent ev) {
fileDeleted0(fo.getName(), fo.getExt()/*, ev.getTime()*/);
}

@Override
public void fileDataCreated(FileEvent ev) {
if (isOurs(ev)) {
if (LOG.isLoggable(Level.FINE)) {
Expand All @@ -1084,6 +1116,7 @@ public void fileDataCreated(FileEvent ev) {
fileCreated0(fo.getName(), fo.getExt()/*, ev.getTime()*/);
}

@Override
public void fileRenamed(FileRenameEvent ev) {
if (isOurs(ev)) {
throw new IllegalStateException("I don't rename anything! " + ev); // NOI18N
Expand Down Expand Up @@ -1124,6 +1157,7 @@ private void fileDeleted0(String name, String ext/*, long time*/) {
} // else ignore
}

@Override
public void fileChanged(FileEvent ev) {
if (isOurs(ev)) {
if (LOG.isLoggable(Level.FINE)) {
Expand All @@ -1150,9 +1184,11 @@ public void fileChanged(FileEvent ev) {
} // else ignore
}

@Override
public void fileFolderCreated(FileEvent ev) {
// ignore
}
@Override
public void fileAttributeChanged(FileAttributeEvent ev) {
// ignore
}
Expand Down Expand Up @@ -1549,7 +1585,7 @@ public void run() {
Map<String, Map<String, Object>> cache = readCache();
String[] names;
if (cache != null) {
names = cache.keySet().toArray(new String[cache.size()]);
names = cache.keySet().toArray(String[]::new);
} else {
FileObject[] children = folder.getChildren();
List<String> arr = new ArrayList<String>(children.length);
Expand All @@ -1572,7 +1608,7 @@ public void run() {
LOG.fine("Strange file encountered in modules folder: " + f);
}
}
names = arr.toArray(new String[0]);
names = arr.toArray(String[]::new);
}
ev.log(Events.MODULES_FILE_SCANNED, names.length);
XMLReader reader = null;
Expand Down Expand Up @@ -1635,12 +1671,12 @@ public void run() {
}
ModuleHistory history = new ModuleHistory(jar, "loaded from " + f); // NOI18N
Boolean reloadableB = (Boolean) props.get("reloadable"); // NOI18N
boolean reloadable = reloadableB != null ? reloadableB.booleanValue() : false;
boolean enabled = enabledB != null ? enabledB.booleanValue() : false;
boolean reloadable = reloadableB != null ? reloadableB : false;
boolean enabled = enabledB != null ? enabledB : false;
Boolean autoloadB = (Boolean) props.get("autoload"); // NOI18N
boolean autoload = autoloadB != null ? autoloadB.booleanValue() : false;
boolean autoload = autoloadB != null ? autoloadB : false;
Boolean eagerB = (Boolean) props.get("eager"); // NOI18N
boolean eager = eagerB != null ? eagerB.booleanValue() : false;
boolean eager = eagerB != null ? eagerB : false;
NbInstaller.register(name, props.get("deps")); // NOI18N
Integer startLevel = (Integer)props.get("startlevel"); // NOI18N
Module m = createModule(jarFile, history, reloadable, autoload, eager, startLevel);
Expand Down
Loading
Loading