From c763b0f6ec0dfa211392ff47ddbea650c503b672 Mon Sep 17 00:00:00 2001 From: Arne Babenhauserheide Date: Thu, 25 Jul 2024 12:17:47 +0200 Subject: [PATCH 1/5] Trigger manual GC earlier MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We now have far more memory than before and file sizes also increased. I’m not sure whether we could just delete the whole gc code, though, and trust the JVM. --- src/freenet/node/MemoryChecker.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/freenet/node/MemoryChecker.java b/src/freenet/node/MemoryChecker.java index 18923380760..cf4acf56671 100644 --- a/src/freenet/node/MemoryChecker.java +++ b/src/freenet/node/MemoryChecker.java @@ -60,7 +60,7 @@ public void run() { else avgFreeMemory.report(freeMemory); - if (avgFreeMemory.countReports() >= 3 && avgFreeMemory.currentValue() < 4 * 1024 * 1024) {// average free memory < 4 MB + if (avgFreeMemory.countReports() >= 3 && avgFreeMemory.currentValue() < 32 * 1024 * 1024) {// average free memory < 32 MB Logger.normal(this, "Reached threshold, checking for low memory ..."); System.gc(); System.runFinalization(); From c958dc48696dcae433d6ad7c1b64fcb782c040e4 Mon Sep 17 00:00:00 2001 From: Arne Babenhauserheide Date: Sun, 28 Jul 2024 11:33:39 +0200 Subject: [PATCH 2/5] Make option getting resilient against requesting ignored options --- src/freenet/config/SubConfig.java | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/freenet/config/SubConfig.java b/src/freenet/config/SubConfig.java index 8a2f5d28abb..7fc66d54d90 100644 --- a/src/freenet/config/SubConfig.java +++ b/src/freenet/config/SubConfig.java @@ -172,7 +172,8 @@ public int getInt(String optionName) { synchronized(this) { o = (IntOption) map.get(optionName); } - return o.getValue(); + // return fallback value for ignored options (null). This avoids breaking plugins which try to get ignored options. + return o == null ? -1 : o.getValue(); } public long getLong(String optionName) { @@ -180,7 +181,8 @@ public long getLong(String optionName) { synchronized(this) { o = (LongOption) map.get(optionName); } - return o.getValue(); + // return fallback value for ignored options (null). This avoids breaking plugins which try to get ignored options. + return o == null ? -1L : o.getValue(); } public boolean getBoolean(String optionName) { @@ -188,7 +190,8 @@ public boolean getBoolean(String optionName) { synchronized(this) { o = (BooleanOption) map.get(optionName); } - return o.getValue(); + // return fallback value for ignored options (null). This avoids breaking plugins which try to get ignored options. + return o == null ? false : o.getValue(); } public String getString(String optionName) { @@ -196,7 +199,8 @@ public String getString(String optionName) { synchronized(this) { o = (StringOption) map.get(optionName); } - return o.getValue().trim(); + // return fallback value for ignored options (null). This avoids breaking plugins which try to get ignored options. + return o == null ? "" : o.getValue().trim(); } public String[] getStringArr(String optionName) { @@ -204,7 +208,8 @@ public String[] getStringArr(String optionName) { synchronized(this) { o = (StringArrOption) map.get(optionName); } - return o.getValue(); + // return fallback value for ignored options (null). This avoids breaking plugins which try to get ignored options. + return o == null ? new String[]{} : o.getValue(); } public short getShort(String optionName) { @@ -212,7 +217,8 @@ public short getShort(String optionName) { synchronized(this) { o = (ShortOption) map.get(optionName); } - return o.getValue(); + // return fallback value for ignored options (null). This avoids breaking plugins which try to get ignored options. + return o == null ? -1 : o.getValue(); } public Option removeOption(String optionName) { From dd1dc38efbcd1f5983cb79ccc750ba54f347cb25 Mon Sep 17 00:00:00 2001 From: Arne Babenhauserheide Date: Sun, 28 Jul 2024 11:39:52 +0200 Subject: [PATCH 3/5] Remove garbage collector meddling. Java is much better at gc now. --- src/freenet/node/MemoryChecker.java | 50 +---------------------------- src/freenet/node/NodeStats.java | 16 +-------- 2 files changed, 2 insertions(+), 64 deletions(-) diff --git a/src/freenet/node/MemoryChecker.java b/src/freenet/node/MemoryChecker.java index cf4acf56671..47bab6829eb 100644 --- a/src/freenet/node/MemoryChecker.java +++ b/src/freenet/node/MemoryChecker.java @@ -14,8 +14,7 @@ public class MemoryChecker implements Runnable { private volatile boolean goon = false; private final Ticker ps; private int aggressiveGCModificator; - private RunningAverage avgFreeMemory; - + public MemoryChecker(Ticker ps, int modificator){ this.ps = ps; this.aggressiveGCModificator = modificator; @@ -48,61 +47,14 @@ public void run() { long totalMemory = r.totalMemory(); long freeMemory = r.freeMemory(); - long maxMemory = r.maxMemory(); Logger.normal(this, "Memory in use: "+SizeUtil.formatSize((totalMemory-freeMemory))); - if (totalMemory == maxMemory || maxMemory == Long.MAX_VALUE) { - // jvm have allocated maximum memory - // totalMemory never decrease, so check it only for once - if (avgFreeMemory == null) - avgFreeMemory = new SimpleRunningAverage(3, freeMemory); - else - avgFreeMemory.report(freeMemory); - - if (avgFreeMemory.countReports() >= 3 && avgFreeMemory.currentValue() < 32 * 1024 * 1024) {// average free memory < 32 MB - Logger.normal(this, "Reached threshold, checking for low memory ..."); - System.gc(); - System.runFinalization(); - - try { - Thread.sleep(10); // Force a context switch, finalization need a CS to complete - } catch (InterruptedException e) { - } - - freeMemory = r.freeMemory(); - avgFreeMemory.report(freeMemory); - } - } - int sleeptime = aggressiveGCModificator; if(sleeptime <= 0) { // We are done ps.queueTimedJob(this, 120 * 250); // 30 sec - return; } else ps.queueTimedJob(this, 120L * sleeptime); - - // FIXME - // Do not remove until all known memory issues fixed, - // Especially #66 - // This probably reduces performance, but it makes - // memory usage *more predictable*. This will make - // tracking down the sort of nasty unpredictable OOMs - // we are getting much easier. - if(aggressiveGCModificator > 0) { - boolean logMINOR = Logger.shouldLog(LogLevel.MINOR, this); - long beforeGCUsedMemory = (r.totalMemory() - r.freeMemory()); - if(logMINOR) Logger.minor(this, "Memory in use before GC: "+beforeGCUsedMemory); - long beforeGCTime = System.currentTimeMillis(); - System.gc(); - System.runFinalization(); - long afterGCTime = System.currentTimeMillis(); - long afterGCUsedMemory = (r.totalMemory() - r.freeMemory()); - if(logMINOR) { - Logger.minor(this, "Memory in use after GC: "+afterGCUsedMemory); - Logger.minor(this, "GC completed after "+(afterGCTime - beforeGCTime)+"ms and \"recovered\" "+(beforeGCUsedMemory - afterGCUsedMemory)+" bytes, leaving "+afterGCUsedMemory+" bytes used"); - } - } } } diff --git a/src/freenet/node/NodeStats.java b/src/freenet/node/NodeStats.java index 986edcc4400..2564af71e1f 100644 --- a/src/freenet/node/NodeStats.java +++ b/src/freenet/node/NodeStats.java @@ -407,21 +407,7 @@ public void set(Integer val) throws InvalidConfigValueException { threadLimit = statsConfig.getInt("threadLimit"); // Yes it could be in seconds insteed of multiples of 0.12, but we don't want people to play with it :) - statsConfig.register("aggressiveGC", aggressiveGCModificator, sortOrder++, true, false, "NodeStat.aggressiveGC", "NodeStat.aggressiveGCLong", - new IntCallback() { - @Override - public Integer get() { - return aggressiveGCModificator; - } - @Override - public void set(Integer val) throws InvalidConfigValueException { - if (get().equals(val)) - return; - Logger.normal(this, "Changing aggressiveGCModificator to "+val); - aggressiveGCModificator = val; - } - },false); - aggressiveGCModificator = statsConfig.getInt("aggressiveGC"); + statsConfig.registerIgnoredOption("aggressiveGC"); myMemoryChecker = new MemoryChecker(node.getTicker(), aggressiveGCModificator); statsConfig.register("memoryChecker", true, sortOrder++, true, false, "NodeStat.memCheck", "NodeStat.memCheckLong", From 52051bee50583e5c15dbc1b9ad6b7c6291a5befe Mon Sep 17 00:00:00 2001 From: Arne Babenhauserheide Date: Sat, 30 Nov 2024 21:47:13 +0100 Subject: [PATCH 4/5] =?UTF-8?q?Kill=20MemoryChecker=20with=20=F0=9F=94=A5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/freenet/node/MemoryChecker.java | 60 ----------------------------- src/freenet/node/NodeStats.java | 23 +---------- 2 files changed, 1 insertion(+), 82 deletions(-) delete mode 100644 src/freenet/node/MemoryChecker.java diff --git a/src/freenet/node/MemoryChecker.java b/src/freenet/node/MemoryChecker.java deleted file mode 100644 index 47bab6829eb..00000000000 --- a/src/freenet/node/MemoryChecker.java +++ /dev/null @@ -1,60 +0,0 @@ -/* This code is part of Freenet. It is distributed under the GNU General - * Public License, version 2 (or at your option any later version). See - * http://www.gnu.org/ for further details of the GPL. */ -package freenet.node; - -import freenet.support.Logger; -import freenet.support.SizeUtil; -import freenet.support.Ticker; -import freenet.support.Logger.LogLevel; -import freenet.support.math.RunningAverage; -import freenet.support.math.SimpleRunningAverage; - -public class MemoryChecker implements Runnable { - private volatile boolean goon = false; - private final Ticker ps; - private int aggressiveGCModificator; - - public MemoryChecker(Ticker ps, int modificator){ - this.ps = ps; - this.aggressiveGCModificator = modificator; - } - - protected synchronized void terminate() { - goon = false; - Logger.normal(this, "Terminating Memory Checker!"); - } - - public boolean isRunning() { - return goon; - } - - public synchronized void start() { - goon = true; - Logger.normal(this, "Starting Memory Checker!"); - run(); - } - - @Override - public void run() { - freenet.support.Logger.OSThread.logPID(this); - if(!goon){ - Logger.normal(this, "Goon is false ; killing MemoryChecker"); - return; - } - - Runtime r = Runtime.getRuntime(); - - long totalMemory = r.totalMemory(); - long freeMemory = r.freeMemory(); - - Logger.normal(this, "Memory in use: "+SizeUtil.formatSize((totalMemory-freeMemory))); - - int sleeptime = aggressiveGCModificator; - if(sleeptime <= 0) { // We are done - ps.queueTimedJob(this, 120 * 250); // 30 sec - } else - ps.queueTimedJob(this, 120L * sleeptime); - - } -} diff --git a/src/freenet/node/NodeStats.java b/src/freenet/node/NodeStats.java index 2564af71e1f..44655108bcb 100644 --- a/src/freenet/node/NodeStats.java +++ b/src/freenet/node/NodeStats.java @@ -125,7 +125,6 @@ final int[] getCounts(final int[] total) { private volatile long maxPingTime; final Node node; - private MemoryChecker myMemoryChecker; public final PeerManager peers; final RandomSource hardRandom; @@ -409,27 +408,7 @@ public void set(Integer val) throws InvalidConfigValueException { // Yes it could be in seconds insteed of multiples of 0.12, but we don't want people to play with it :) statsConfig.registerIgnoredOption("aggressiveGC"); - myMemoryChecker = new MemoryChecker(node.getTicker(), aggressiveGCModificator); - statsConfig.register("memoryChecker", true, sortOrder++, true, false, "NodeStat.memCheck", "NodeStat.memCheckLong", - new BooleanCallback(){ - @Override - public Boolean get() { - return myMemoryChecker.isRunning(); - } - - @Override - public void set(Boolean val) throws InvalidConfigValueException { - if (get().equals(val)) - return; - - if(val) - myMemoryChecker.start(); - else - myMemoryChecker.terminate(); - } - }); - if(statsConfig.getBoolean("memoryChecker")) - myMemoryChecker.start(); + statsConfig.registerIgnoredOption("memoryChecker"); statsConfig.register("ignoreLocalVsRemoteBandwidthLiability", false, sortOrder++, true, false, "NodeStat.ignoreLocalVsRemoteBandwidthLiability", "NodeStat.ignoreLocalVsRemoteBandwidthLiabilityLong", new BooleanCallback() { From fdc6f6f5ebaddf35f7a16fa0ff10a66acf689b5e Mon Sep 17 00:00:00 2001 From: Arne Babenhauserheide Date: Sat, 30 Nov 2024 21:54:38 +0100 Subject: [PATCH 5/5] Remove unused aggressiveGCModificator --- src/freenet/node/NodeStats.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/freenet/node/NodeStats.java b/src/freenet/node/NodeStats.java index 44655108bcb..2b9d50570a0 100644 --- a/src/freenet/node/NodeStats.java +++ b/src/freenet/node/NodeStats.java @@ -283,10 +283,6 @@ public void shouldUpdate(){ final StringCounter preemptiveRejectReasons; final StringCounter localPreemptiveRejectReasons; - // Enable this if you run into hard to debug OOMs. - // Disabled to prevent long pauses every 30 seconds. - private int aggressiveGCModificator = -1 /*250*/; - // Peers stats /** Next time to update PeerManagerUserAlert stats */ private long nextPeerManagerUserAlertStatsUpdateTime = -1;