Hello all,

 

The short version:

More CPU-Cycles or Higher Memory Usage: which is more preferred to have more (or less) of?

A class with the least amount of, or as few as possible, fields and having all attributes which can be calculated from these variables whenever the #getAttribute() is called (on the fly). Or, a class that only does these calculations up front or when an update to any of the fields occur and store each attribute of an Object in a field (keeping in mind changes to any of the fields is correctly passed on to others).

Or, something in between... But where is this 'sweet spot'?

 

Understandably this discussion has probably been done many times before. However, I couldn't find a recent discussion on it through Google or the Search function on this forum. That is why I am just posting it here, I probably just used the wrong search queries so point me to the discussion where this has been discussed before.

---

How I came to this question (Really, just background, only to support how I came to this question):

Recently I have been working on a mod which is getting along nicely but slowly. Slowly, as I currently am working on the foundation... A.k.a. over-implementing the util-package. As the mod relies heavily on multi-block structures I have been focussing on that a lot to make it as simple as possible when I get to implementing them. But along the way I have been slowed down a lot by of instances of the question I have asked above.

As someone who has learned and trained to always try to be as memory-efficient as possible, I tend to go for more CPU cycles. Let me give you the code I currently have. You don't need to go through it all, I will point out stuff with snippets further down.

[spoiler=Whole bunch of code]

This class is mainly used for multi-block structures. Don't let the lack of function-documentation fool you, the documentation is placed in the interfaces the classes implement.

package nl.scribblon.riftcraft.util;

import net.minecraft.block.Block;
import net.minecraft.tileentity.TileEntity;
import nl.scribblon.riftcraft.util.iplace.ILocationRC;
import nl.scribblon.riftcraft.util.iplace.IRelativeLocationRC;

/**
* Created by Scribblon for RiftCraft.
* Date Creation: 28-8-2014
*/
public class RelativeLocation implements IRelativeLocationRC {

    public static final RelativeLocation ROOT = new RelativeLocation(0,0,0);

    private boolean isInterDimensional;
    private double x, y, z;

    public RelativeLocation(boolean isInterDimensional, double x, double y, double z) {
        this.isInterDimensional = isInterDimensional;
        this.x = x;
        this.y = y;
        this.z = z;
    }

    public RelativeLocation(double x, double y, double z) {
        this(false, x, y, z);
    }

    public RelativeLocation(ILocationRC location1, ILocationRC location2) {
        this.isInterDimensional = location1.getDimensionID() == location2.getDimensionID();
        this.x = location2.getX() - location1.getX();
        this.y = location2.getY() - location1.getY();
        this.z = location2.getZ() - location1.getZ();
    }

    /*_*********************************************************************************************************
     * Vector Functions
     */
    @Override
    public IRelativeLocationRC getInverse() {
        return new RelativeLocation(this.isInterDimensional, -this.x, -this.y, -this.z);
    }

    @Override
    public IRelativeLocationRC sum(IRelativeLocationRC relativeLocation) {
        return new RelativeLocation(
                this.x + relativeLocation.getXShift(),
                this.y + relativeLocation.getYShift(),
                this.z + relativeLocation.getZShift());
    }

    @Override
    public IRelativeLocationRC subtract(IRelativeLocationRC relativeLocation){
        return new RelativeLocation(
                this.x - relativeLocation.getXShift(),
                this.y - relativeLocation.getYShift(),
                this.z - relativeLocation.getZShift()
        );
    }

    /*_*********************************************************************************************************
     * Literal getters of various kinds
     */
    @Override
    public double getXShift() {
        return this.x;
    }

    @Override
    public double getYShift() {
        return this.y;
    }

    @Override
    public double getZShift() {
        return this.z;
    }

    @Override
    public int getIntXShift() {
        return (int) Math.floor(this.x);
    }

    @Override
    public int getIntYShift() {
        return (int) Math.floor(this.y);
    }

    @Override
    public int getIntZShift() {
        return (int) Math.floor(this.z);
    }

    @Override
    public boolean isInterDimensional() {
        return this.isInterDimensional;
    }

    /*_*********************************************************************************************************
     * Relative getters of various kinds
     */
    @Override
    public double getShiftedXTo(ILocationRC location) {
        return location.getX() + this.x;
    }

    @Override
    public double getShiftedYTo(ILocationRC location) {
        return location.getY() + this.y;
    }

    @Override
    public double getShiftedZTo(ILocationRC location) {
        return location.getZ() + this.z;
    }

    @Override
    public int getShiftedIntXTo(ILocationRC location) {
        return (int) this.getShiftedXTo(location);
    }

    @Override
    public int getShiftedIntYTo(ILocationRC location) {
        return (int) this.getShiftedYTo(location);
    }

    @Override
    public int getShiftedIntZTo(ILocationRC location) {
        return (int) this.getShiftedZTo(location);
    }

    @Override
    public ILocationRC getILocationRelativelyFrom(ILocationRC location) {
        return new Location(location.getWorld(), this.getShiftedXTo(location), this.getShiftedYTo(location), this.getShiftedZTo(location));
    }

    @Override
    public Block getBlockRelativelyFrom(ILocationRC location) {
        return this.getILocationRelativelyFrom(location).getBlockAtLocation();
    }

    @Override
    public TileEntity getTileEntityRelativelyFrom(ILocationRC location) {
        return this.getILocationRelativelyFrom(location).getTileEntityAtLocation();
    }

    /*_*********************************************************************************************************
     * Auto-Generated things
     */
    @Override
    public boolean equals(Object o) {
        if (this == o) return true;
        if (!(o instanceof RelativeLocation)) return false;

        RelativeLocation that = (RelativeLocation) o;

        if (isInterDimensional != that.isInterDimensional) return false;
        if (Double.compare(that.x, x) != 0) return false;
        if (Double.compare(that.y, y) != 0) return false;
        if (Double.compare(that.z, z) != 0) return false;

        return true;
    }

    @Override
    public int hashCode() {
        int result;
        long temp;
        result = (isInterDimensional ? 1 : 0);
        temp = Double.doubleToLongBits(x);
        result = 31 * result + (int) (temp ^ (temp >>> 32)); //I don't understand this... '
        temp = Double.doubleToLongBits(y);
        result = 31 * result + (int) (temp ^ (temp >>> 32)); //I know these are bit shifts and are basically applying *2 to the value
        temp = Double.doubleToLongBits(z);
        result = 31 * result + (int) (temp ^ (temp >>> 32)); //But the '^' part and the need to do this... I don't...
        return result;
    }
}

This is basically RelativeLocation with a bit more information about what is allowed to be there.

package nl.scribblon.riftcraft.util;

import net.minecraft.block.Block;
import nl.scribblon.riftcraft.util.imulti.IMultiTiledMaster;
import nl.scribblon.riftcraft.util.iplace.IRelativeLocationRC;

import java.util.Arrays;

/**
* Created by Scribblon for RiftCraft.
* Date Creation: 29-8-2014
*/
public class RelativeStructureBlock extends RelativeLocation implements Comparable {

    public static final RelativeStructureBlock ROOT = new RelativeStructureBlock(0,0,0);

    private Block[] allowedStructureParts;  //Should maybe make this dynamic (Set)... But as far as I can see this is not needed.
                                            //Might be changed to OreDictionary compatible things... Maybe it is already...

    //Only for testing and defining a ROOT position is this here
    private RelativeStructureBlock(double x, double y, double z) {
        super(false, x,y,z);
        this.allowedStructureParts = null;
    }

    public RelativeStructureBlock(boolean isInterDimensional, double x, double y, double z, Block... allowedStructureParts) {
        super(isInterDimensional, x, y, z);
        this.allowedStructureParts = allowedStructureParts;
    }

    public RelativeStructureBlock(double x, double y, double z, Block... allowedStructureParts) {
        super(x, y, z);
        this.allowedStructureParts = allowedStructureParts;
    }

    public Block[] getAllowedStructureParts(){
        return this.allowedStructureParts;
    }

    public boolean isBlockSupported(Block block) {
        if (allowedStructureParts.length <= 0) return false;

        for (Block allowedPart : allowedStructureParts) {
            if (allowedPart.equals(block))
                return true;
        }

        return false;
    }

    public boolean isBlockSupportedRelativeTo(IMultiTiledMaster master) {
        return this.isBlockSupported(this.getBlockRelativelyFrom(master.getLocation()));
    }

    /*_*********************************************************************************************************
     * Auto-Generated things
     */
    @Override
    public boolean equals(Object o) {
        if (this == o) return true;
        if (!(o instanceof RelativeStructureBlock)) return false;
        if (!super.equals(o)) return false;

        RelativeStructureBlock that = (RelativeStructureBlock) o;

        if (!Arrays.equals(allowedStructureParts, that.allowedStructureParts)) return false;

        return true;
    }

    @Override
    public int hashCode() {
        int result = super.hashCode();
        result = 31 * result + Arrays.hashCode(allowedStructureParts);
        return result;
    }

    /*_*********************************************************************************************************
     * Semi-Auto-Generated
     */
    @Override
    public int compareTo(Object o) throws ClassCastException {
        if(!(o instanceof IRelativeLocationRC))
            throw new ClassCastException("An IRelativeLocationRC implementations was expected.");

        IRelativeLocationRC that = (IRelativeLocationRC) o;

        if(this.getYShift() < that.getYShift()) return -1;

        if(this.getYShift() > that.getYShift()) return +1;

        if(this.getZShift() < that.getZShift()) return -1;

        if(this.getZShift() > that.getZShift()) return +1;

        if(this.getXShift() > that.getXShift()) return -1;

        if(this.getXShift() < that.getXShift()) return +1;

        return 0;

    }
}

This is the class which will contain all the information which defines a multi-block structure. This class will be implemented in the block that will become the master block as a public static. The current implementation does not support rotations but well... That is something for later.

It uses a SortedSet implementation (TreeSet) to ensure that the blocks are checked in the order I intended them to be. Although I am aware of the log(n) speed of the add, remove, look-up, of this implementation. I intend to add the parts in order, it is just when I mess up it will ensure it still will.

package nl.scribblon.riftcraft.util;

import nl.scribblon.riftcraft.util.imulti.IMultiTiledMaster;
import nl.scribblon.riftcraft.util.iplace.IRelativeStructure;

import java.util.Set;
import java.util.TreeSet;

/**
* Created by Scribblon for RiftCraft.
* Date Creation: 29-8-2014
*/
public class RelativeStructure implements IRelativeStructure {

    private TreeSet<RelativeStructureBlock> parts;

    public RelativeStructure(RelativeStructureBlock... parts){
        this.parts = new TreeSet<RelativeStructureBlock>();
        for(RelativeStructureBlock part : parts)
            this.parts.add(part);
    }

    @Override
    public Set<RelativeStructureBlock> getParts() {
        return this.parts;
    }

    @Override
    public RelativeStructureBlock getRoot() {
        return parts.subSet(RelativeStructureBlock.ROOT, RelativeStructureBlock.ROOT).first();
    }

    @Override
    public boolean structureSupportsMaster(IMultiTiledMaster master) {
        return this.getRoot().isBlockSupported(master.getBlockType());
    }

    @Override
    public boolean isStructureCorrectFrom(IMultiTiledMaster master) {
        if(!this.structureSupportsMaster(master)) return false;

        for(RelativeStructureBlock structureBlock : this.parts) {
            if(!structureBlock.isBlockSupportedRelativeTo(master))
                return false;
        }
        return true;
    }
}

The leveled variant of the RelativeStructure. The class that has given me most headaches about the cpu vs memory question.

The data-type of the 'levels' field has changed a lot. It could have been easily an array, but somewhere I needed it to be dynamic in some way. But that is not the biggest thing.

It is the amount of 'for' loops it implement to calculate the other properties of the Object. But I will go on further down below.

package nl.scribblon.riftcraft.util;

import nl.scribblon.riftcraft.util.imulti.IMultiTiledMaster;
import nl.scribblon.riftcraft.util.iplace.ILeveledRelativeStructure;

import java.util.*;

/**
* Created by Scribblon for RiftCraft.
* Date Creation: 29-8-2014
*/
public class LeveledRelativeStructure implements ILeveledRelativeStructure {

    private ArrayList<RelativeStructure> levels;

    public LeveledRelativeStructure(RelativeStructure... levels){
        this.levels = new ArrayList<RelativeStructure>(levels.length);
        for(RelativeStructure level : levels)
            this.levels.add(level);
    }

    @Override
    public Set<RelativeStructureBlock> getParts(int level) {
        return this.levels.get(level).getParts();
    }

    @Override
    public boolean isStructureCorrectFrom(IMultiTiledMaster master, int toLevel) {
        return this.detectLevel(master) == toLevel;
    }

    @Override
    public int detectLevel(IMultiTiledMaster master) {
        if(!this.structureSupportsMaster(master)) return INVALID;

        for(Map.Entry<RelativeStructureBlock, Integer> set : this.getLevelMap(ROOT_LEVEL, levels.size()).entrySet()) {
            if(!set.getKey().isBlockSupportedRelativeTo(master))
                return set.getValue() - 1;
        }
        return INVALID;
    }

    @Override
    public Map<RelativeStructureBlock, Integer> getLevelMap(int fromLevel, int toLevel) {
        LinkedHashMap<RelativeStructureBlock, Integer> levelMap = new LinkedHashMap<RelativeStructureBlock, Integer>();

        for(int levelValue = fromLevel; levelValue < toLevel && levelValue < this.levels.size(); ++levelValue) {
            for(RelativeStructureBlock block : this.getParts(levelValue)) {
                levelMap.put(block, levelValue);
            }
        }

        return levelMap;
    }

    @Override
    public Set<RelativeStructureBlock> getParts() {
        TreeSet<RelativeStructureBlock> all = new TreeSet<RelativeStructureBlock>();
        for(RelativeStructure structureLevel : this.levels)
            all.addAll(structureLevel.getParts());

        return all;
    }

    @Override
    public RelativeStructureBlock getRoot() {
        return this.levels.get(ROOT_LEVEL).getRoot();
    }

    @Override
    public boolean structureSupportsMaster(IMultiTiledMaster master) {
        return this.levels.get(ROOT_LEVEL).structureSupportsMaster(master);
    }

    @Override
    public boolean isStructureCorrectFrom(IMultiTiledMaster master) {
        return this.isStructureCorrectFrom(master, levels.size());
    }
}

I am always open for suggestions, improvements and such.

 

 

 

package nl.scribblon.riftcraft.util;

<snip>
public class LeveledRelativeStructure implements ILeveledRelativeStructure {

    private ArrayList<RelativeStructure> levels;

    <snip>

    @Override
    public int detectLevel(IMultiTiledMaster master) {
        if(!this.structureSupportsMaster(master)) return INVALID;

        for(Map.Entry<RelativeStructureBlock, Integer> set : this.getLevelMap(ROOT_LEVEL, levels.size()).entrySet()) {
            if(!set.getKey().isBlockSupportedRelativeTo(master))
                return set.getValue() - 1;
        }
        return INVALID;
    }

    @Override
    public Map<RelativeStructureBlock, Integer> getLevelMap(int fromLevel, int toLevel) {
        LinkedHashMap<RelativeStructureBlock, Integer> levelMap = new LinkedHashMap<RelativeStructureBlock, Integer>();

        for(int levelValue = fromLevel; levelValue < toLevel && levelValue < this.levels.size(); ++levelValue) {
            for(RelativeStructureBlock block : this.getParts(levelValue)) {
                levelMap.put(block, levelValue);
            }
        }

        return levelMap;
    }

    @Override
    public Set<RelativeStructureBlock> getParts() {
        TreeSet<RelativeStructureBlock> all = new TreeSet<RelativeStructureBlock>();
        for(RelativeStructure structureLevel : this.levels)
            all.addAll(structureLevel.getParts());

        return all;
    }

    <snip>
}

 

The point of my question being. The methods not snipped out in the class above can be implemented in two ways. One way is that it is a simple getter with the fields being calculated in the constructor.

Or like this. However, TreeSet has a O(log(n)) on add, remove and contains. For traverse this is not much of an issue.

It only becomes a thing in #getParts(), even though I am not planning on making massive structures where the Big-O can become an issue. And this method will be only called rarely. But even then the O(log(n)) will have the highest increase in time per element in the lower numbers...

The other methods however (#detectLevel(..) and #getLevelMap(..) will be called every time the structure requires to check its structure onBlockBreak (once, if the structure breaks it will not check further), but when the upgrade is on the way this is probably called every time a block is placed or broken in the vicinity of the structure. Or an update is invoked with a what of a certain item.

As someone who has learned and trained to use as less fields, the way I do it here is 'the correct way as I have been toughed'. However, of course, this is not the only way. As I know tick-lag is a big thing... I am certain memory-usage is one too. And I don't know where the right middle-ground is in this case.

 

So, a slightly different formulated question from above. Where is the 'best place to be' in the 'CPU-Cycles vs Memory Usage' spectrum when modding for minecraft?

 

My gratitude in advance,

Hi

 

My general approach is to code it in a way that is easy to understand and maintain.  Then I do some performance testing, find the slow parts, refactor them, and test again.

 

So I reckon the answer to your question is "test it under typical operating conditions and find out".

 

Some general rules I personally apply-

1) Simple and easy to understand; no fancy tricks

2) Minimise network traffic.

3) Don't worry about memory unless it's more than a MB or so.

4) Don't worry about doing any optimisations on CPU usage at all, beyond choosing the right type of data structure for the operations you're doing (say LinkedList vs HashMap)

5) Always ask the question "will the user notice the difference?"

6) Test, then optimise.  Don't "optimise" first.

 

-TGG

 

 

 

 

 

 

 

 

Disclaimer: I haven't been modding minecraft for very long

 

TheGreyGhost gave a typical Java answer.

And that's also something you have to keep in mind, minecraft is written in java.

It's not in C++ with its handy destructors, pointers, cpu & memory control

Instead it is in java which is a generalized language with less possibilities towards your particular application & optimalization but easier to code in e.g. it is meant for not worrying about cpu or ram.

So in a sense it suffices to simply follow normal procedure of only saving necessary things and calculating the rest, avoiding calculations in for loops or in much triggered methods etc.

The way I generally approach my minecraft methods is if I see they'll be called a lot without needing it is by simply throwing in an if statement first.

Other than that, there really isn't a need to worry about cpu or memory unless you really see it.

Just write your classes and after you're done, just go over them and improve where possible.

Optimization is never bad, but it shouldn't be done too much. TheGreyGhost tips are actually useful.

In Minecraft, optimization wouldn't make much difference except in some cases:

1- You're doing it stupidly, such as calling code everywhere unnecessary.

2- You're doing something big, which is very rarely the case.

3- The user is using over 150 mods, which is not your problem.

 

Answering your question, there is no this-or-that simple answer, it depends on a lot of things that neither is more important than the other, this means you shouldn't worry except if it lags when testing or if it is too obvious. Optimization should be the last thing to do, it is time-consuming and some things might get deprecated overtime, in which case all the time used in optimizing them gets wasted.