TeamPorcupine / ProjectPorcupine

Project Porcupine: A Base-Building Game...in Space!
GNU General Public License v3.0
485 stars 278 forks source link

[BUG] Update is run at 5 fps #1383

Closed Raketfart closed 8 years ago

Raketfart commented 8 years ago

I was debugging why furniture animation was so laggy. I figured out that deltatime in lua and furniture update was 0.2 second. This seems to be because TimeManager calls updates 5 times a second, from what I could see.

This means that even though we see 60 fps in the counter, nothing updates at more than 5 FPS.

Raketfart commented 8 years ago

Alright - I found the reason. World.cs calls furniture.update in TickFixedFrequency() which is 5FPS instead of TickEveryFrame() at 60 FPS.

I can see some logic behind this choice, but then we should rename OnUpdate functions to make it clear that it's run at less fps. Also, maybe we can have both update() and slowupdate() for furniture. Doors and other furniture needs to update the animation faster than 5 FPS.

jaselito commented 8 years ago

I agree, though that would probably mean providing more animation frames for certain sprites

Raketfart commented 8 years ago

It would be possible, but not a requirement. Animations will run fine if update is more frequent

jaselito commented 8 years ago

Indeed, but without enough frames that'd be a waste of update cycles (i.e. why updating if the same frame is going to be shown). Of course, things other than animation could also benefit from a higher update frequency.

Raketfart commented 8 years ago

I agree. That's why I think maybe we offer both options to lua. A fast and a slow update. Things like oxygen compressor can choose to update slow and doors can update fast

sboigelot commented 8 years ago

We should keep fast update only for visual things. Be very restrictive about it.

Raketfart commented 8 years ago

I agree. But door animation is based on openness, which changes very fast in lua. I don't know of a good way to fix this other than leaving it up to Lua to chose

Raketfart commented 8 years ago

maybe we can determine if furniture is in camera viewport, and only fastupdate those in view. Then we keep control in c# and lua doesn't have to care.

sboigelot commented 8 years ago

@Raketfart that's a great idea

Raketfart commented 8 years ago

It seems like renderer.isVisible will tell if an object is in cameraview. I have no idea if that's "expensive" to do each frame. If it is, we could probably check who is visible, when we move/zoom the camera.

sboigelot commented 8 years ago

I have no idea about the performance involved in that. We could also use a camera WorldToViewportPoint... but ideally we should split the world in chunck and calculate this only chunck by chunk and not on a tile basic. Unless there is a clever way to calculate the amount of tiles visible on the screen based on the camera coordinate and the zoom.

TomMalbran commented 8 years ago

If Lua is the slow part, could we instead have a faster interval that only runs the animation in C# and lerps between the previous value and the new one and never call Lua?

If changing the sprite too much is also an issue, we will need to check if is visible or something, but also reduce the changing rate when we are zoomed out a lot.

sboigelot commented 8 years ago

The original reason why we pace-out the update was not lua being slow, but the code being useless to run every update (quest, temperature, gaz...). Reduce the usage of cpu intensive task that would not change anything for the player.

It is ok to have an extra fast call for the animation, and it makes sense as it is a visual thing. Controlling the animation via c# is ok too but would reduce the flexibility of it if we are not careful.

kd7uiy commented 8 years ago

@sboigelot Out of curiosity, what kind of change do you think would require animation to be done via LUA code? I kind of think a state machine would be good, where the state is updated by LUA, but the individual frames are processed by C#, would be best, but I haven't looked in to it a whole lot...

Raketfart commented 8 years ago

I've been wondering a bit about this. The current version isn't any better than updating everything every frame. We've just push the problem to only appear every 5th frame, so we have these lagspikes, which is no better than constantly doing all the work IMO. lagspikes

If we want this to be better, we should distribute the work to be done instead of delaying it. So frame 1 is furniture update, frame 2 is temperature, frame 3 is power, frame 4 is something else....

We should still have some furniture that can update every frame, either when in camera view or by using two update functions.

Raketfart commented 8 years ago

Sorry I'm just rambling on. I did some measuring, and found that furniture update in the starting base "costs" 1ms. Adding around 20 powergenerators and oxygen generators raises the costs to 4ms. 20 more and it goes to 6 ms. Adding many more raised it to around 10 ms for furn updates. That means that we will quickly reach a point where furniture updates will cause lag spikes 5 times a second with the current system.

Raketfart commented 8 years ago

Alright! I know this is premature optimization, but I tried to isolate furniture update with 780 pieces of furniture - about half of those with lua update scripts. The peaks reached 40 ms for code, which was mostly lua. Instead of "resting" for 9 frames, and the working hard on the 10th., I tried to distribute the update between all 10 frames, which seemed to work fine, bringing the code back to around 10 ms.

I don't think this is something we should do now, but knowing that we have the option is nice. I think this proves that furniture update is expensive and should be limited. I'll try to test some options for solving the door problem.

Raketfart commented 8 years ago

This is turning into my personal notesbook. I've made a save with 900 doors. This is approximately what it could handle and still run at 60 FPS: 500 doors with the simplest lua call I could make. 160 doors with current OnUpdate_Door. 110 doors with current OnUpdate_Door + OnUpdate_Leak_Airlock.

By moving furniture between a visible and invisible list, we can update visible furniture every frame, and invisible furniture in the fixed frequency.

I'll try to clean up my tests, and use test the invisible list method on the real master.

mikejbrown commented 8 years ago

Remember there's now a wiki page for useful saves. :wink:

Raketfart commented 8 years ago

Good point. I'll put the relevant parts of this on the wiki, when a solution is found. I hope that the solution will be unnessesary for modders to know about, because it won't affect how they should write xml & lua. But internally and for later points of optimization, it would be useful info.

sboigelot commented 8 years ago

I've added a fast update in the PR above to solve the issue for now. We can still improve it with a better solution later but at least this will make the door open smooooothly :D

TomMalbran commented 8 years ago

I think that it might be hard to add in TimeManager some sort of buffer system that could handle calls on different frames.

Raketfart commented 8 years ago

@sboigelot That's one solution - looks good, and fixes the problem for now. If we end up with a solution controlled in TimeManager/World, we would probably remove the extra lua update. I feel it's confusing to have two update functions with different elapsed, and could lead to lots of frustrating debugging.

@TomMalbran It might be hard - but I think it will be worth if, if we can support 5 times as many furniture updates. We don't have to do it now.

TomMalbran commented 8 years ago

For furniture: TimeManager could have x amount of lists with furnitures (or callbacks), where each lists represents a different time tick. On each tick TimeManager calls the update function of each furniture (or callbacks) in the current list. When a a furniture is created you add it to the list with the smallest count. When is deleted, you remove it. Each furniture could also have a weight indicating the time it takes to update (a furniture with no lua update will have a lower weight, than one with it). We then use this weight to calculate a total weight per list and keep the lists weights similar.

Raketfart commented 8 years ago

Yes thats the sort of thing i was thinking.

If furniture has no lua, it can go in the nonupdatefurnitures list. I dont think we have to measure the cost of lua, but dividing them in seperate lists like that would be very efficient.

And i already made a working test with moving visible furniture in and out of the list that updates every frame.

TomMalbran commented 8 years ago

The weight/cost is just for flexibility. But it could be something like 0 if it requires no update at all, 1 if the update is only in C#, 5 if it has Lua code calls on update, and it could be higher if it does some intensive calculation. (Or some numbers that make more sense). The weight could also be higher if the furniture has an animation that doesn't require to run every frame and, if replacing the graphics also slow things down like Lua (no idea if they do in Unity).

Raketfart commented 8 years ago

Oh yeah, now i get you. That sounds like a reasonable use of weight. I thought you meant measuring the actual time it took.

I think the renderer takes care of not updating objects outside the camera view, so that part shouldn't be a problem.

TomMalbran commented 8 years ago

I did said time, and we can go and measure it. But we can just use values depending on what the update function does.