ID:2689316
 
(See the best response by Kaiochao.)
Problem description:
I have searched high and low for optimization tips in old Developer Help posts and even in Design Philosophy and Tutorials/Snippets. I did not think that something as common as lag would have so little information about how to prevent it. The only helpful things I've really found are the thing about World.tick which Ter13 (Whom I consider quite a reliable source) said that you should avoid overusing that since u don't wanna distribute EVERYTHING across ticks. And there was something about Find() being 4x faster than in but I honestly don't really know how to make use of that.

What are solutions to common mistakes in optimization that I may be able to incorporate?

P.S. At the moment, I'm getting some insane overhead.
https://prnt.sc/13wflgz
The first question in optimization is what code you're trying to optimize. The place to start is to get profiling data on your procs using the built-in profiler.

What's some of the code you suspect is running slow? What sorts of things in your game might not be scaling well?
The only helpful things I've really found are the thing about World.tick which Ter13 (Whom I consider quite a reliable source) said that you should avoid overusing that since u don't wanna distribute EVERYTHING across ticks.

I said what now? Can you give me a link? Old me and now me might be getting ready to fight.
In response to Ter13
Ter13 wrote:
The only helpful things I've really found are the thing about World.tick which Ter13 (Whom I consider quite a reliable source) said that you should avoid overusing that since u don't wanna distribute EVERYTHING across ticks.

I said what now? Can you give me a link? Old me and now me might be getting ready to fight.

http://www.byond.com/forum/post/151655
http://www.byond.com/forum/post/2345869
Kenryuto1 wrote:
P.S. At the moment, I'm getting some insane overhead.
https://prnt.sc/13wflgz

I'm not sure where the crazy high real-time values are coming from, but real-time is not actually a measure of overhead or CPU use. It includes intentional delays such as sleep(), input(), etc. which aren't actually lag.

Show some code of the procs that have the most total CPU. It'd also help to include the code of the procs they call with the most self CPU.

The only really general tip I have for how to optimize something that needs it is... do less work. Loop over fewer things, do less per loop, etc.

But it's always worth reiterating: don't optimize things that don't need to be. Optimization tends to make code more complex and harder to work with. And as with any change to code, it takes development time and will probably introduce bugs.
In response to Lummox JR
        GF/Game_Crasher
name = "Lag Causer"
suffix = "description"
Effect(mob/M)
..()
Click()
for(var/obj/Moves/Game_Crashers/gc in usr.Moves)if(!gc.Active)return
..()
aoe=usr.loc.loc //I'm getting area here
Game_Crasher()
var
tmp/obj/Crasher = null
tmp/area/aoe
verb
Game_Crasher()
set category = "Fighting"
set background = 1
if(usr.Fighting||usr.CantMove||usr.Busy||CD||usr.Stamina <= 2100) return
view(25,usr)<<"<i>[usr.name] grabs on to the barriers of the game...</i>"
usr.Fighting++
usr.CantMove+=3.001 //this will paralyze the user for 4 seconds
dmgOutput = (usr.Muscle+usr.GameCrasherStat)*3
spawn(40)
aoe<<"<b><i>[usr] yanks the x, y, and z planes together thus crashing the game!</i></b>"
for(var/turf/T in aoe)
if(aoe == /area/NonPKarea)return
spawn(2)if(prob(70))T.Transform(pick("dirtydirt","wetwater","densemountainturf"),usr)
// spawn(1)if(prob(30))T.Transform("dirtydirt",usr)
// spawn(1)if(prob(45))T.Transform("wetwater",usr)
// spawn(1)if(prob(30))T.Transform("densemountainturf",usr)
// if(world.tick_usage > 85) lagstopsleep()

This was at the top of the list for insane real time. Problem is that the only spawns I have in are spawn(40) and spawn(2). So that shouldn't make the number as high as it is should it? Also, this takes the highest cpu when it finishes running.
In response to Kenryuto1
I've been told that looping over the turfs in an area actually loops over every turf in existence and checks if its loc is the area you're looping over. This is pretty bad if your "aoe" is much smaller than the size of the world.

You're repeating the aoe type check for every turf. You could have checked it once before the loop and avoided the loop entirely. But this is not going to be the main cause for issues.

Unless T.Transform() sleeps, it doesn't need to be spawned. If you want the 2 delay, it's constant, so put it before the loop.

Looping over every turf in the world is not going to crash your game no matter how large the world is.

turf.Transform() is definitely the issue.
In response to Kaiochao
To clarify, despite the names of the techniques and variables, this never crashes the game. It just makes it a slight lag spike while its being done, and the it lags for the rest of the uptime until reboot after the technique has been long ended.
In response to Kaiochao
Kaiochao wrote:
You're repeating the aoe type check for every turf. You could have checked it once before the loop and avoided the loop entirely. But this is not going to be the main cause for issues.
How would I pull this off?
Unless T.Transform() sleeps, it doesn't need to be spawned. If you want the 2 delay, it's constant, so put it before the loop.
When I don't spawn atleast 0 or 1, the proc simply doesnt work at all. It will transform one tile, then wait for that tile to wear off when takes several minutes, and the transform the next tile. Not the intended result at all. The only reason I spawn 2 instead of 0 is because i just had a hope that a little bit more of a delay would reduce the terrible lag that comes from it all.
Looping over every turf in the world is not going to crash your game no matter how large the world is.
So then is it worth fixing?
turf.Transform() is definitely the issue.
That proc has the second most high real time but do you think that the only reason this procs real time is so high is because transform has high real time?
In response to Kenryuto1
How would I pull this off?
// before
for(var/turf/T in aoe)
if(aoe == /area/NonPKarea)return

// after
if(aoe != /area/NonPKarea)
for(var/turf/T in aoe)

Before: In the best case, it's checked after reaching the first relevant turf. In the worst case (which is the typical case, you're not in the NonPKarea), it's needlessly checked for every turf after that.
After: In any case, it's checked only once, before any turfs are considered. In the best case, there is no loop.
(edited to add best/worst case)

It will transform one tile, then wait for that tile to wear off when takes several minutes, and the transform the next tile.
Then spawn is necessary, since you want multiple procs which take time to start at the same time. A potentially cheaper alternative is to call a proc with set waitfor = FALSE that calls Transform, or to add set waitfor = FALSE to Transform itself if no caller will ever need to wait for Transform. If you do use waitfor, you'll have to move the sleep(2) to before the loop if you want it.

So then is it worth fixing?
What?

That proc has the second most high real time but do you think that the only reason this procs real time is so high is because transform has high real time?
I'm guessing the realtime of the proc itself comes from the sum of the realtime of each Transform called in it, which makes realtime even less relevant of a performance measure. Again, realtime isn't actually an indicator of the lag caused by a proc.
In response to Kenryuto1
Kenryuto1 wrote:
Ter13 wrote:
The only helpful things I've really found are the thing about World.tick which Ter13 (Whom I consider quite a reliable source) said that you should avoid overusing that since u don't wanna distribute EVERYTHING across ticks.

I said what now? Can you give me a link? Old me and now me might be getting ready to fight.

http://www.byond.com/forum/post/151655
http://www.byond.com/forum/post/2345869

Just as a PSA, What I was saying was not to never distribute behavior over multiple ticks. What I was saying was: You should only do this where it is absolutely critical. It's about instantaneous response to actions by players. You want as little time between when a player tries to do something and when they get feedback. This comment was in no way a comment on optimization. I was actually saying, more or less: "This is not a magic pill to mak gam fast. Don't spray it all over your source code blindly.". It's... Really concerning that this has been taken as a general rule of thumb to "mak fast" when I was really trying to stop people from making the assumption that there is a general "mak fast" rule of thumb. Sleeping incurs scheduler debt. If you use it improperly, you can wind up with more overhead, not less.
Here's an example of a proc with ridiculously high realtime but just as ridiculously low actual CPU usage, just to emphasize again that you really shouldn't care about the realtime measure:
mob/verb/high_realtime_low_cpu()
sleep(1000)

(The profiler won't show the actual realtime until it finishes, 100 seconds later.)
In response to Ter13
Just as a PSA, What I was saying was not to never distribute behavior over multiple ticks. What I was saying was: You should only do this where it is absolutely critical. It's about instantaneous response to actions by players. You want as little time between when a player tries to do something and when they get feedback. This comment was in no way a comment on optimization. I was actually saying, more or less: "This is not a magic pill to mak gam fast. Don't spray it all over your source code blindly.". It's... Really concerning that this has been taken as a general rule of thumb to "mak fast" when I was really trying to stop people from making the assumption that there is a general "mak fast" rule of thumb. Sleeping incurs scheduler debt. If you use it improperly, you can wind up with more overhead, not less.

This is exactly how I interpreted your comment and I am thankful for it because before I saw it, I actually had already systematically gone through all of my source code and sprayed it under damn near every for() loop and while() loop that I could find. After recently reading your comments, I have slowly started undoing all that and simultaneously trying to leave it where it would actually be useful. Thanks Ter13
In response to Kaiochao
I realize that. It's been repeated enough to be impressed on me. It's not just high numbers concerning me though. There is actual lag caused by it that literally lasts for the remainder of the server's runtime and i would like to not have that.
In response to Kenryuto1
turf.Transform() is definitely the issue. What's it doing?
In response to Kaiochao
turf
proc
Transform(Turf,mob/Owner)
set background=1
if(loc:type == /area/Safe_Zone)return
switch(Turf)
if("dirtydirt")
icon_state = Turf
density = 0
opacity = 0
Duration = Owner.GameCrashStat*5.5
Type = "Environment"
if("densemountainturf")
icon_state = Turf
density = 1
opacity = 0
Duration = Owner.GameCrashStat*5.5
Type = "Environment"
if("wetwater")
icon_state = Turf
density = 0
opacity = 0
Duration = Owner.GameCrashStat*5.5
Type = "Environment"
Creator = Owner
while(Duration > 0)
Duration -= 5
sleep(10)
Creator = initial(Creator)
Duration = initial(Duration)
Type = initial(Type)
Owner = initial(Owner)
icon = initial(icon)
icon_state = initial(icon_state)
density = initial(density)
opacity = initial(opacity)
var list/initial_values = list("creator"=...,"duration"=...)


And then reapplying them at the end via that list instead of relying on initial()
In response to Kenryuto1
Best response
            while(Duration > 0)
Duration -= 5
sleep(10)

This is what's causing the lag over time. Every turf in the aoe is running this loop. Every second, every turf in the aoe does something and then sleeps for another second. It might not seem like much, but multiplied by the number of turfs you have, it is apparently enough to cause lag that "lasts for the remainder of the server's runtime" (but in theory should only last as long as the while() lasts).

If you do the math, the actual sleep duration is 10 * Duration / 5.
In other words, you sleep by 10 for every time you can remove 5 from Duration, so you can just sleep once by that amount.

One large optimization you can make is to only sleep once. The thing to notice is that the duration of the sleep doesn't differ between the turfs, so you don't need each turf to sleep separately (each call to sleep has its own overhead, so it's best to minimize the calls). There's actually a lot of work being done in Transform per turf that all of the turfs are repeating exactly.

Assuming there aren't any overrides of Transform() that drastically change what's being done.

Here's one way to decrease the amount of repeated identical work:
// This replaces the for(var/turf/T in aoe) loop in Game_Crasher().

// Don't do anything for certain types of areas. This doesn't need to be checked in any loops or by the turfs.
if(istype(aoe, /area/NonPKarea) || istype(aoe, /area/Safe_Zone))
return

// Add all turfs in the aoe to the appropriate lists.
var/list/transformed_turfs = new
var/list/turfs_by_type = list(
"dirtydirt" = list(),
"densemountainturf" = list(),
"wetwater" = list())

for(var/turf/turf in aoe)
// We want to reset them all later. This is what we'll loop over to do that.
transformed_turfs += turf

// Randomly choose a type for this turf, and add the turf to the list for that type in turfs_by_type.
turfs_by_type[pick(turfs_by_type)] += turf

// And this happens to every affected turf too, so might as well do it here.
turf.Creator = usr

// Do all the stuff dependent on type, separately.
// By having separate loops over specific things, we avoid overhead from repeated switch()/if().
for(var/turf/turf as () in turfs_by_type["dirtydirt"])
turf.TransformIntoDirtyDirt()

for(var/turf/turf as () in turfs_by_type["wetwater"])
turf.TransformIntoWetWater()

for(var/turf/turf as () in turfs_by_type["densemountainturf"])
turf.TransformIntoDenseMountainTurf()

// This only needs to happen once.
sleep(10 / 5 * 5.5 * usr.GameCrashStat)

for(var/turf/turf as () in transformed_turfs)
turf.UndoTransform()

and the turfs get these procs:
turf
proc/TransformIntoDirtyDirt()
Type = "Environment"
icon_state = "dirtydirt"
density = FALSE
opacity = FALSE

proc/TransformIntoWetWater()
Type = "Environment"
icon_state = "wetwater"
density = FALSE
opacity = FALSE

proc/TransformIntoDenseMountainTurf()
Type = "Environment"
icon_state = "densemountainturf"
density = TRUE
opacity = FALSE

proc/UndoTransform()
Creator = initial(Creator)
Type = initial(Type)
icon_state = initial(icon_state)
density = initial(density)
opacity = initial(opacity)

Instead of having lag every second until Transform ends, you'll just have some lag at the start and at the end. This lag is mostly unavoidable. I'd say it's appropriate in this case to have it spread over ticks.

Also, the "as ()" in the loops is to disable type checking when looping with a type. All the lists in here only contain turfs, so it saves work by not bothering to filter the list to turfs.
In response to Kaiochao
For more efficiently changing the icon_state, density, and opacity of many objects, you can use mutable appearances:
proc/TransformIntoDirtyDirt()
Type = "Environment"
var/mutable_appearance/m = new(src)
m.icon_state = "dirtydirt"
m.opacity = FALSE
m.density = FALSE
appearance = m

// etc.

It might even be possible to reuse mutable appearances across all the turfs that need the same one.
Is there a way to make it so that I can use
for(var/turf/T in anything)

without looping through every tile in the game.
Page: 1 2