Remove no longer needed garbage collector meddling. #971
Conversation
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.
We absolutely should, especially because that code is not guaranteed to actually trigger a GC. The only way to force the JVM to do a garbage collection is by exhausting all memory; when the |
|
@Bombe I now removed that code and also the aggressiveGC option that was only used there. Also Hyphanet no longer dies during startup when an ignored option is used during startup (but I’m not happy about the error reporting: do we have a way to report this very loudly?). |
ArneBab
changed the title
src/freenet/node/MemoryChecker.java
Outdated
| @@ -14,8 +14,7 @@ public class MemoryChecker implements Runnable { | |||
| private volatile boolean goon = false; | |||
| private final Ticker ps; | |||
| private int aggressiveGCModificator; | |||
| private RunningAverage avgFreeMemory; | |||
|
|
|||
|
|
|||
There was a problem hiding this comment.
The field aggressiveGCModificator can be removed, it’s always -1 now.
src/freenet/node/MemoryChecker.java
Outdated
| avgFreeMemory.report(freeMemory); | ||
| } | ||
| } | ||
|
|
||
| int sleeptime = aggressiveGCModificator; | ||
| if(sleeptime <= 0) { // We are done |
There was a problem hiding this comment.
aggressiveGCModificator going away means this also doesn’t need an if anymore.
| @@ -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 :) | |||
There was a problem hiding this comment.
The comment can also be removed. 🙂
src/freenet/node/NodeStats.java
Outdated
| } | ||
| },false); | ||
| aggressiveGCModificator = statsConfig.getInt("aggressiveGC"); | ||
| statsConfig.registerIgnoredOption("aggressiveGC"); | ||
|
|
||
| myMemoryChecker = new MemoryChecker(node.getTicker(), aggressiveGCModificator); |
There was a problem hiding this comment.
aggressiveGCModificator should also be removed from this class, including the comment above its declaration. 🙂
| 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(); |
There was a problem hiding this comment.
Can we replace the ternaries with the more descriptive Map.getOrDefault?
There was a problem hiding this comment.
I tried: not without increasing the complexity, because map returns an option which does not have a simple constructor from just a value.
There was a problem hiding this comment.
Ah, yeah. I kind of forgot about the mess the constructor of Option is 😞
Spoiler:
Option(SubConfig config, String name, ConfigCallback<T> cb, int sortOrder, boolean expert, boolean forceWrite,
String shortDesc, String longDesc, DataType dataType) {Well then let's stick with the ternaries!
src/freenet/node/MemoryChecker.java
Outdated
There was a problem hiding this comment.
Does this class still serve any purpose (aside from scheduling itself 😄) after removing pretty much all of its code?
There was a problem hiding this comment.
Haha, that is an excellent observation! 😄 Kill it with 🔥! 🥳
There was a problem hiding this comment.
It’s killed with 🔥 ☺
Good catch!
|
Thank you for the reviews! Merged! |
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.