OpenBW / BWAPI4J

BWAPI wrapper for Java
GNU Lesser General Public License v3.0
24 stars 9 forks source link

Internal cache objects #45

Closed adakitesystems closed 6 years ago

adakitesystems commented 6 years ago

Inspired by comments made by @MasterFocus and @dgant on Discord, I would like to discuss caching all possible "computed" objects/lists such as BW#getUnits(Player).

Example idea from @dgant's PurpleWave bot repo: https://github.com/dgant/PurpleWave/blob/master/src/Performance/Cache.scala

Possible Java solution (untested):

public class Cache<T> {

    private int lastFrameUpdate;
    private T data;
    private final Callable<T> updateFunction;

    public Cache(final Callable<T> updateFunction) {
        this.lastFrameUpdate = -1;
        this.updateFunction = updateFunction;
    }

    public T get(final int currentFrameCount) {
        if (this.lastFrameUpdate < currentFrameCount) {
            try {
                this.data = this.updateFunction.call();
            } catch (Exception e) {
                throw new RuntimeException(e);
            }

            this.lastFrameUpdate = currentFrameCount;
        }

        return this.data;
    }

}

In the specific case of BW#getUnits(Player), @MasterFocus suggested the following:

MasterFocus: perhaps you could get a stream from all units and use Collectors.groupingBy() to create a map like Map< Player , Unit >
MasterFocus: getUnits(somePlayer) would just have to do map.get(somePlayer) (or something like that)

@dgant suggested to remove int currentFrameCount from the parameter list. To do this, we may need a FrameCount class that could be passed around? Something where FrameCount is an interface instantiated as the subclass FrameCountSetter used by whichever class does the updating (e.g. BW.java) and store the FrameCount type in all relevant Cache objects?

MasterFocus commented 6 years ago

This is what I meant:

// private variable:
private Map<Player, List<PlayerUnit>> testMap;
// populate inside a method executed before onFrame():
this.testMap = this.units.values().stream()
        .filter(u -> u instanceof PlayerUnit)
        .map(PlayerUnit.class::cast)
        .collect(Collectors.groupingBy(PlayerUnit::getPlayer));

Or if we wanted to keep them stored as Units:

// private variable:
private Map<Player, List<Unit>> testMap;
// populate inside a method executed before onFrame():
this.testMap = allUnits().stream()
        .filter(u -> u instanceof PlayerUnit)
        .collect(Collectors.groupingBy(u -> ((PlayerUnit)u).getPlayer()));

Supposedly, this allows us to rewrite getUnits(Player) as:

public List<PlayerUnit> getUnits(Player) {
    return this.testMap.get(player);
}

(please note this is untested and should be checked for mutability and possible NPEs as well)

The mere fact of doing this and populating the map before onFrame() should probably be enough to speed up multiple calls of getUnits(Player).

adakitesystems commented 6 years ago

Closed with Pull Request https://github.com/OpenBW/BWAPI4J/pull/46