C_Map.GetPlayerMapPosition Memory Usage
 

Anyone else having memory issues with C_Map.GetPlayerMapPosition? I updated some map coords code to use the new functions and noticed that the addons memory usage grows every update. By commenting out other code I was able to narrow the problem down to this function so I know it is the problem. I had it setup like this.


and also tried this.


I also tried moving the variables outside of the function but nothing changed. Anyone know how to fix this. The addon worked with no issues with the old functions on live.

It's just how Lua handles tables, and it's been a constant issue I've had with Blizzard's trend with their APIs lately. Every time an API is called that returns a table, it takes up a small chunk of memory. This can add up quickly, especially in combat log events and OnUpdate. It doesn't matter if you assign it to a variable or not, just the act of calling the function does this.

Specificly, C_Map.GetPlayerMapPosition() is generating this new table every time it's called. The memory eventually gets freed when Lua determines it needs to, but it's bad practice to rely on its garbage collection as it can cause random lag spikes.

When you use it that way make sure that mapID has a value.
mapID can be undefined while using portals.

Iirc, an instance will have a mapID but not have the Position mixin which contains the :GetXY() function.

This is getting off topic. The discussion is how to counteract the memory impact of calling C_Map.GetPlayerMapPosition() frequently.

Arguments are reserved(?)
 

In the old days of scanning for nameplates JamPlates used the method of declaring variables as arguments to reserve memory.

Edit: Think there was a misunderstanding. The global function was localized because it bothered me.

That's just localizing the function, because local functions are something along the lines of ~25% faster than global functions in terms of executions.

The problem with map position is that some position-tracking addons uses it OnUpdate which was fine when it was two simple numbers. But the new API causes that code to create a new table every frame. Sure, those tables end up being collected as garbage sooner or later, but it's rather annoying and may scare users into thinking something is wrong with your addon.

Blizzard dev who created those API's said to not worry about memory usage.

That's the point though, it's not just memory. When the garbage collection is called more often, it eats up cycles and becomes a CPU problem.

I never said anything but just so it clear to everyone the function that elcius posted works well.

Does anyone know how to scale the worldmap properly?
If you just simple SetScale, you won't be able to switch zones by click on different part of the maps.

I tried the code, the memory usage went up each time you switch zones on the worldmap.

Unless you can measure an actual impact from that, its still not a problem.

If you are extremely worried about the memory consumption from the Map APIs, you could use my Map library HereBeDragons which offers table-free position APIs (althought thats not the reason I made it that way, its just the same API the library had in 7.x already) - and no, it also doesn't just wrap the Blizzard Map API to "hide" the tables. :)

I found this in LeatixPlus by using "RunScript", and it does not produce memory waste.

That's way worse. It's just not attributing the memory usage to the addon. Now it's spending time lexing and compiling the string into byte code, which then becomes garbage. And then you pay the same price for the C_Map.GetPlayerMapPosition call.

You can. Initiate a garbage collection manually and watch your FPS.

Memory usage is not an issue. Having giant tables of data actually saves cpu, especially if you’re not calling things in combat. One fine example of memory/cpu efficiency is Details. In terms of combat FPS, Details clearly, objectively, wins over Skada or Recount.

Memory waste is bad. An occasional wasted object is sloppy but not usually an issue. However, dumping a table every frame is what we call a “memory leak”. Lua’s garbage collection is the least efficient part of the entire implementation. It’s like a garbage inception. You want to avoid it at all costs, but with hundreds of different addon authors, it’s inevitable. A blip in FPS every hour or so is fine, maybe every 30 minutes if I’m running a bunch of addons, but memory leaks trigger one every few minutes, as often as several a minute.

Having Blizzard give you a function that directly generates waste every time you use it for data that several addons want on an OnUpdate resolution is probably the worst thing any Blizzard dev has ever done for this game. It’s atrocious, disgusting, reeks of novice coding. I have enjoyed all the mixin stuff I’ve seen popup over the last few years. Someone on the dev team has lots of love for Lua. I sincerely hope these wasteful issues were an oversight, a bug, or done from someone new that will be fixed by someone senior.

Because a manual full run of GC is actually the same as the incremental GC that runs in the background, right? :)

Please, go ahead and do show incontrovertible evidence without manually interfering with the garbage collector, then we can continue. All these arguments are grasping.

This is curiosity, not a "gang-up".

Unless there is some significant optimisation (happy to be educated), even incremental GC will be called more often as there is more G being produced to C.

GC time is proportionate to the complexity of the garbage. These are incredibly simple tables.

Which begs the question, why use a table in a language that allows multiple returns?

I feel like I must be missing some simple point, which wouldn't be the first time :o.

I'd guess it's towards normalization of data. For instance, look at C_Map.GetMapLinksForMap, it returns the same 2D vector table but as a field of a subtable in a collection of tables. In that case, multiple returns don't help, but containing them in a table keeps the data consistent. Vector 2D is likely the most contentious given that it is just two fields, but it scales better the more fields the type contains.

I remain to be convinced that it's normalisation ;).

This issue has come up a lot in #wowuidev over the last several months. There is a WoW UI dev in there who kindly answers most of our questions and takes note of issues.

A lot of different people have argued this point with him, and I think I can quickly summarize his two main arguments for those curious:

• They've greatly improved garbage collection performance recently, and have monitored the performance involved in using tons of small, disposable tables under the new GC implementation. It is an incredibly small performance hit.
• Tables help normalize UI implementation and are especially useful in cases where the number of returns is variable. The other thing seems to be preference. It seems like the UI devs prefer the syntax where you pull down a table of results and use the individual parts you need rather than doing 'local _, _, thingA, _, _, _, _, thingB = SomeRandomApi()'.

I'm just paraphrasing here, but this is what I recall. In any case, it seemed like an issue they were -not- willing to budge on. One person was very adamant that they should introduce a function to return this one argument he was interested in because he polled it very frequently and was now required to pull down a table (Spell name, maybe? I forget), and the UI dev was not willing to entertain the possibility of introducing specialized functions to bypass the table returns. He was fairly adamant that there would be a near zero performance impact due to the speed of simple table disposal.

The only annoying part here is that it is basically impossible for us to independently verify their claims because as noted in previous posts, you can't really perform a partial GC on demand and record cycles spent or reliably compare the results. The only ones who can really know the exact impact of these changes are Blizzard themselves.

I can actually agree with this part. I have used a table to get function returns before, but it was a reusable table in a custom pool function. I have always not liked using an underscore as a universal dump variable. It has caused many minor issues over the course of addon development over the years. The wall of shame link in my signature exposes why it’s an issue.

However, relying on supposed efficiency to excuse inefficiency isn’t a great coding practice. It’s like tossing bottles at a campsite because the park you’re at has a world class janitor army. That’s still littering.

Great analogy!

Did they define "tons" or give a timeframe in which "tons" were created/disposed? This is an API for public consumption and test lab or even Blizzard UI conditions rarely mimic all the real world uses.

I have IRC logs, so let me just paste the whole conversation - the one I was thinking of when I posted that above. TheDanW is the UI dev. This conversation occurred in December 2017, after BFA alpha was out but when addons were disabled so most of us were just watching the interface files for diffs in order to speculate on what changes needed to be made.

https://paste2.org/wZGNWyGv

There's at least a few more instances of this type of conversation, but I don't really want to dig up his entire post history for people to take apart outside context, just to post a bit so you can see Blizzard's reasoning on this. Also, he mentions in one other conversation that if people are interested, he could add support to a few frequently called functions so that somebody can pass their own table as an argument in order to be reused. Might be a decent thing for somebody to suggest for GetPlayerMapPosition, though I'm not sure how viable it is due to it being a mixin (Vector2D) return.

P.S. I realize "Dec 18 17:43:56 <Simca> all new API since WoD return tables, with a few exceptions" this is quite hyperbolic. There are certainly a large number of tables returned by API functions since WoD, but there are more than 'a few exceptions'. Not sure why I decided to say it like that in retrospect.

Edit: Also, he mentions elsewhere that the optimizations for the garbage collector he is talking about were changes that went Live with Patch 7.0.3.

We're in C++ land now, bois :banana:

The blz just don't use it themselves. We only need x, y, and it throw us a table with tons of the methods, this is not all about the GC, the Mixin system fill those methods into a table with lots of the re-hash cost, and mostly we don't really use them.

Thanks to elcius, I don't need to deal with it.:banana: