Ender IO Zoo

Ender IO Zoo

962k Downloads

General BakedQuad Render Improvement

Speiger opened this issue ยท 18 comments

commented

Thanks for all the base description when you make an issue
But its not fitting for my things...

I readed the EnderIO stuff and heared about some issues EnderIO had with to many Conduits causing lag...
(Direwolf20 Server Series) you should know which version that is...

Well after reading the code i noticed what could be an issue...
You can improve the rendering speed by caching already existed cases of renders.

What i mean is basicly a hashMap:
static Map<ConduitEntry, List> quadStorage = new HashMap<ConductEntry, List>();
That would allow you guys to say: Hey i already rendered in this case so i am picking it from the storage and save time....

Why am i suggesting this? When i started making the Renderer for IC2Classic cables and not caching the existing render cases and placed a tower of 10x50x10 cables it was so laggy that i was not even able to look at it without fps going down to 10... Sometimes even my game crashed because of a memory leak... After using a cached system for a cable renderer the FPS went down the first time i saw each case & went right back up... And my memory seemt to stableize too...

And the good thing is: You can define what a condut entry is...
and it can be very complex or very easy but i can tell you for sure the idea is working great and i have no issues at all...

Here is my most complex CaseEntry for example so far:

    public static class MultiLuminatorEntry
    {
        int meta;
        WireColor color;
        Set<EnumFacing> lamps;
        Set<EnumFacing> connect;
        boolean active;
        int hashCode;

        public MultiLuminatorEntry(Set<EnumFacing> lampSides, Set<EnumFacing> connectSide, int damage, WireColor wirecolor, boolean activated)
        {
            meta = damage;
            color = wirecolor;
            lamps = lampSides;
            connect = connectSide;
            active = activated;
            hashCode = Objects.hash(meta, color, lamps, connect, active);
        }

        @Override
        public int hashCode()
        {
            return hashCode;
        }

        @Override
        public boolean equals(Object obj)
        {
            if(obj instanceof MultiLuminatorEntry)
            {
                MultiLuminatorEntry part = (MultiLuminatorEntry)obj;
                return part.meta == meta && part.active == active && part.color == color && part.lamps.equals(lamps) && part.connect.equals(connect);
            }
            return false;
        }
    }

Maybe this helps you guys improving massusage of EnderIO

commented

That'd be a good approach if those attributes were primitives. But they mostly are Map<EnumFacing, DyeColor>, Set, and so on. So refactoring will not help at all, every access must be changed by hand to call the value setter. (refactoring would change "attribute.put(key, value)" to "getAttribute().put(key, value)" instead of the needed "setAttribute(key, value)")

However, I spend 3 days going through the conduits and I think I now got a version of the "compute cache key on the fly" that should be fast enough. It takes at least 15-20 times longer than the original, but as the original already was extremely fast, I believe we can live with it.

commented

there are "readonly" implementations for maps, sets, vectors and lists

but okay you got a solution, thats fine :)

commented

Baked models are already heavily cached. But if you can implement a better cache key than "identity + last change" for the conduits, feel free to PR it. Without looking it up, it needs these fields for the cache key:

has item conduit node
has energy T1 conduit node
has energy T2 conduit node
has energy T3 conduit node
has redstone conduit node
has fluid T1 conduit node
has fluid T2 conduit node
has fluid T3 conduit node
has oc conduit node

has item conduit connection to (up, down, north, west, east, south)
has energy T1 conduit connection to (up, down, north, west, east, south)
has energy T2 conduit connection to (up, down, north, west, east, south)
has energy T3 conduit connection to (up, down, north, west, east, south)
has redstone conduit connection to (up, down, north, west, east, south)
has fluid T1 conduit connection to (up, down, north, west, east, south)
has fluid T2 conduit connection to (up, down, north, west, east, south)
has fluid T3 conduit connection to (up, down, north, west, east, south)
has oc conduit connection to (up, down, north, west, east, south)

has connection plate to (up, down, north, west, east, south)

item conduit IO to (up, down, north, west, east, south) is (n/a, in, out, in+out)
energy T1 conduit IO to (up, down, north, west, east, south) is (n/a, in, out, in+out)
energy T2 conduit IO to (up, down, north, west, east, south) is (n/a, in, out, in+out)
energy T3 conduit IO to (up, down, north, west, east, south) is (n/a, in, out, in+out)
fluid T1 conduit IO to (up, down, north, west, east, south) is (n/a, in, out, in+out)
fluid T2 conduit IO to (up, down, north, west, east, south) is (n/a, in, out, in+out)
fluid T3 conduit IO to (up, down, north, west, east, south) is (n/a, in, out, in+out)

item conduit channel to (up, down, north, west, east, south) is (16 colors)
redstone conduit channel to (up, down, north, west, east, south) is (16 colors)
fluid T3 conduit channel to (up, down, north, west, east, south) is (16 colors)
oc conduit channel to (up, down, north, west, east, south) is (16 colors)

redstone conduit is (lit/not lit)
===
250 bits

and the state of conduits that are added by other mods

Oh, and collecting the cache key must not take longer than taking the TE's hash and adding the TE's iteration number to it.

commented

Well i do not have the time to make a custom PR...
Just throwing ideas in... And yeah already rendered chunks are already havyly storing baked quads but as soon chunks get rerendered your baked quads get requested again...
So yeah... But caching already rendered cases (all the time) saves the longer the time runs more and more time... So yeah....

Edit:
You can make as complex keys as you want. You can even say multi type combinations...
As i said: that is my most complex one because i do not need something more complex....
But it depends on how you create the KeyClass...

commented

It seems we're not quite on the same page here.

Conduit models are cached. The cache key is "te.hashcode() + te.getSerial()", the later is incremented every time a conduit is changed. (btw, in its painted form the cache key is the blockstate it is painted with.)

So you can re-render your chunks as often as you want, the model will be taken from the cache. Only if you change a conduit its model will be recreated.

And yes, I see that there's the possibility for a better cache key. I outlined what needs to be put into it above. But, the cache key needs to be computed when the model is rendered. At the moment we are very fast there, basically combining two integers. A more complex cache key, which would allow models to be shared between conduits, would take ages to be collected from all parts that make up a conduit. It would take nearly as much time as re-building the model, making caching become nonsensical.

So the solution would be for the conduits to have the cache key be stored in the TE and be updated with the state of the conduit bundle every time there is some change. So it will already exist when the rendering happens. However, this would mean changing all the conduit code. Every single bit of code that changes some state in the bundle or any of the conduits would need to be changed to also update the cache key.

Which I'd happily do, if this was my full-time job and we had a release date some time next year.

commented

So you can re-render your chunks as often as you want, the model will be taken from the cache. Only if you change a conduit its model will be recreated.

Yeah thats what i meant. It would cache every case of the change. As long it has created the case already it will ask the storage: Hey do i have rendered your case already if not here you get it and then save it for me...

And yeah its a lot of work... But Redstone Conduits have that issue for example...
It was only a help suggestion... I never said: Hey you have to do this!
I am just rewriting ic2c and take already months finishing it... So yeah i know how much work that can be...

commented

Caching every possible variant of conduits would likely fill up even the largest PC's RAM. You underestimate just HOW MANY possible iterations there are of conduits.

commented

@tterrag1098.... Ic2Wires have a shit tone of cases too... We talk about over 32k cases.... So yeah i know how much cases can be... But it does not eat up the ram...
Als what you can do is say: Make a Time Based Cache... So if a case does not get used for a long time (lets say 15 minutes) its get thrown out of the cache... So it is only keeping what it is needed...

commented

i have no idea how that cache works at all but i feel an itch to throw my two cents on such a lively cache
you should try to add some object recycling while you are on it, the GC can be a bitch sometimes
and when you add ageing, it becomes even more dynamic
so maybe try to keep as many shared attributes as possible and do not drop whole objects for the sake of new ones

it may make the cache a bit slower as atribute modification can take longer than instantiation but the overall game performance may increase

commented

We have somewhat like 1809251394333065553493296640760748560207343000000000000000000000000000000000 cases. And we DO HAVE A CACHE. And we have a size limit on the cache (500 entries). And a time limit (10 minutes).

PS: That number is not "keyboard mashing" but real. Loss of precision possible because of size.

About keeping attributes: It is a bit hard to have dynamic data in a baked model. After all, those quads are baked and changing them "on demand" would defeat the whole purpose of baking them.

commented

Just a question... (Brainstorming and i do not assume it will save a lot but i am interested. (my idea is already closed))
what if you make for each conductor its each own storage & build then a bakedquad list based on the types & the cases of the cable.
So you would have (lets say 6 types) 6 types * 36 cases (just basic math not very detailed) instead of a way higher count with combining cases... Simply splitting parts... Bulding a list of combos could decreases the cases right? (Talking only about connecting & active/deactive state & type state... Not fluid color or facades...)

commented

It's not 6 cases per side. The side-specific values are:

has item conduit connection to (up, down, north, west, east, south)
has energy T1 conduit connection to (up, down, north, west, east, south)
has energy T2 conduit connection to (up, down, north, west, east, south)
has energy T3 conduit connection to (up, down, north, west, east, south)
has redstone conduit connection to (up, down, north, west, east, south)
has fluid T1 conduit connection to (up, down, north, west, east, south)
has fluid T2 conduit connection to (up, down, north, west, east, south)
has fluid T3 conduit connection to (up, down, north, west, east, south)
has oc conduit connection to (up, down, north, west, east, south)

has connection plate to (up, down, north, west, east, south)

item conduit IO to (up, down, north, west, east, south) is (n/a, in, out, in+out)
energy T1 conduit IO to (up, down, north, west, east, south) is (n/a, in, out, in+out)
energy T2 conduit IO to (up, down, north, west, east, south) is (n/a, in, out, in+out)
energy T3 conduit IO to (up, down, north, west, east, south) is (n/a, in, out, in+out)
fluid T1 conduit IO to (up, down, north, west, east, south) is (n/a, in, out, in+out)
fluid T2 conduit IO to (up, down, north, west, east, south) is (n/a, in, out, in+out)
fluid T3 conduit IO to (up, down, north, west, east, south) is (n/a, in, out, in+out)

item conduit channel to (up, down, north, west, east, south) is (16 colors)
redstone conduit channel to (up, down, north, west, east, south) is (16 colors)
fluid T3 conduit channel to (up, down, north, west, east, south) is (16 colors)
oc conduit channel to (up, down, north, west, east, south) is (16 colors)

redstone conduit is (lit/not lit)

That's 37 bits, or 137438953472 different cases if we fold the bits for the tiered conduits (which are technically completely different conduits).

commented

And to repeat myself. The caching is not the problem. We have a working caching system that is working well.

The problem is computing a cache key that allows us to re-use the same cache entry for (a) different conduits and (b) the same conduit after it changes more than once.

I can build that cache key very easily. That object would have the fields I outlines above. However, collecting those values for a conduit would take considerable time. So doing it when being asked for the quads would be very idiotic because it would slow down rendering for all conduits all the time. At the moment, providing a baked model for a conduit that has not changed since the last time it was rendered in so fast that I have problems measuring that time.

So that cache key has to be part of the internal state of the tile entity and has to be kept up-to-date every time the conduit is changed. For that to be implemented, the whole structure of the conduit system has to be changed and all involved classes have to be changed to either update the cache key in addition to their internal data or, even better, to only store their state in the cache key structure. I'd love to do that, but I do not have the time for it. My professional estimate would be something like 40-100 hours of work plus testing.

commented

just one more explanation for me

  1. the time to get the information reuired to bake the model does take too long? (doing updates on the visual is producing lag)
  2. or the time to assemble a cachekey representing the internal data takes too long? (first appearance of the model and renaming when changed creates lag)
commented

(als unsubscribed)

commented

Closing Because for me its done.

commented

Let me put this in pseudo code:

Current:

getModel() {
    if (hasConduitChanged() || !isModelCached()) {
        return cachedModel;
    } else {
        bakeModel();
        cacheModel();
        return model;
    }
}

Proposed:

getModel() {
    if (doWeHaveAModelThatLooksTheSameInTheCache()) {
        return cachedModel;
    } else {
        bakeModel();
        cacheModel();
        return model;
    }
}

The problem is that doWeHaveAModelThatLooksTheSameInTheCache() can be implemented two ways. One is relatively fast to implement but takes ages to execute. The other one takes ages to implement but would run very fast.

bakeModel() is as fast as possible. Nothing to optimize here. Pre-building parts of the model and combining them would theoretically be possible and faster than building them from scratch, but the number of possible ways is either enormous, or the parts would be so tiny that caching them would not be faster than rebuilding (caching 4 quads is not faster than making 4 quads).

commented

i think i have an overview of the possible ways, let me try open a third one ... it's kind of a dirty one ... codewise ...

you can make all your hash-relevant attributes private and use refactoring to give every attribute a getter and a setter methode
in each set methode you could then drop in a updateHash call
this way the hash is updated only on change of relevant data (you can also add a delta check to reduce this even further)

and the usage of refactoring should make the changes in other classes a mild breeze, maybe only a class 1 tornado at worst :)

PS i am yet on a weekend with few iternet access, i may answer a bit delayed