Thread Tools Display Modes
06-15-18, 05:39 PM   #1
thomasjohnshannon
A Theradrim Guardian
 
thomasjohnshannon's Avatar
AddOn Author - Click to view addons
Join Date: Sep 2009
Posts: 68
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.

Lua Code:
  1. local mapID = C_Map.GetBestMapForUnit("player")
  2. local px, py = C_Map.GetPlayerMapPosition(mapID, "player"):GetXY()

and also tried this.

Lua Code:
  1. local px, py = C_Map.GetPlayerMapPosition(C_Map.GetBestMapForUnit("player"), "player"):GetXY()

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.
__________________
Thomas aka Urnn
  Reply With Quote
06-15-18, 05:53 PM   #2
SDPhantom
A Pyroguard Emberseer
 
SDPhantom's Avatar
AddOn Author - Click to view addons
Join Date: Jul 2006
Posts: 2,308
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.
__________________
WoWInterface AddOns
"All I want is a pretty girl, a decent meal, and the right to shoot lightning at fools."
-Anders (Dragon Age: Origins - Awakening)

Last edited by SDPhantom : 06-15-18 at 05:56 PM.
  Reply With Quote
06-15-18, 09:38 PM   #3
elcius
A Cliff Giant
AddOn Author - Click to view addons
Join Date: Sep 2011
Posts: 75
Lua Code:
  1. local MapRects = {};
  2. local TempVec2D = CreateVector2D(0,0);
  3. function GetPlayerMapPos(MapID)
  4.     local R,P,_ = MapRects[MapID],TempVec2D;
  5.     if not R then
  6.         R = {};
  7.         _, R[1] = C_Map.GetWorldPosFromMapPos(MapID,CreateVector2D(0,0));
  8.         _, R[2] = C_Map.GetWorldPosFromMapPos(MapID,CreateVector2D(1,1));
  9.         R[2]:Subtract(R[1]);
  10.         MapRects[MapID] = R;
  11.     end
  12.     P.x, P.y = UnitPosition('Player');
  13.     P:Subtract(R[1]);
  14.     return (1/R[2].y)*P.y, (1/R[2].x)*P.x;
  15. end
  Reply With Quote
06-17-18, 12:40 PM   #4
siweia
A Flamescale Wyrmkin
 
siweia's Avatar
AddOn Author - Click to view addons
Join Date: Jul 2011
Posts: 126
Code:
	local mapID, position
	local formatText = ": %.1f, %.1f"

	hooksecurefunc(WorldMapFrame, "OnMapChanged", function(self)
		mapID = self:GetMapID()
	end)

	local function CoordsUpdate(player, cursor)
		local cx, cy = CursorCoords()
		if cx and cy then
			cursor:SetFormattedText(MOUSE_LABEL..formatText, 100 * cx, 100 * cy)
		else
			cursor:SetText(MOUSE_LABEL..": --, --")
		end

		position = C_Map.GetPlayerMapPosition(mapID, "player")
		if not position or (position.x == 0 and position.y == 0) then
			player:SetText(PLAYER..": --, --")
		else
			player:SetFormattedText(PLAYER..formatText, 100 * position.x, 100 * position.y)
			wipe(position)
		end
	end
  Reply With Quote
06-17-18, 01:02 PM   #5
Rilgamon
Premium Member
 
Rilgamon's Avatar
Premium Member
AddOn Author - Click to view addons
Join Date: Sep 2009
Posts: 822
Lua Code:
  1. local mapID = C_Map.GetBestMapForUnit("player")
  2. local px, py = C_Map.GetPlayerMapPosition(mapID, "player"):GetXY()
When you use it that way make sure that mapID has a value.
mapID can be undefined while using portals.
__________________
The cataclysm broke the world ... and the pandas could not fix it!
  Reply With Quote
06-17-18, 03:41 PM   #6
VincentSDSH
Non-Canadian Luzer!
 
VincentSDSH's Avatar
AddOn Author - Click to view addons
Join Date: Jun 2006
Posts: 350
Lua Code:
  1. local px, py = C_Map.GetPlayerMapPosition(mapID, "player"):GetXY()
Iirc, an instance will have a mapID but not have the Position mixin which contains the :GetXY() function.
__________________
AddonsExecutive Assistant User Configurable To-Do ListLegible Mail Choose the Font for Your Mail
  Reply With Quote
06-18-18, 05:54 AM   #7
SDPhantom
A Pyroguard Emberseer
 
SDPhantom's Avatar
AddOn Author - Click to view addons
Join Date: Jul 2006
Posts: 2,308
This is getting off topic. The discussion is how to counteract the memory impact of calling C_Map.GetPlayerMapPosition() frequently.
__________________
WoWInterface AddOns
"All I want is a pretty girl, a decent meal, and the right to shoot lightning at fools."
-Anders (Dragon Age: Origins - Awakening)
  Reply With Quote
06-18-18, 10:58 AM   #8
jeruku
A Cobalt Mageweaver
 
jeruku's Avatar
AddOn Author - Click to view addons
Join Date: Oct 2010
Posts: 223
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.

Lua Code:
  1. -- get/set mapID somewhere/somehow
  2. local function GetPlayerMapPos(px, py)  --  declare variables as arguments
  3.     -- some stuff here
  4.     px, py = C_Map.GetPlayerMapPosition(mapID, "player"):GetXY()
  5.     -- more stuff
  6. end
__________________
"I have not failed, I simply found 10,000 ways that did not work." - Thomas Edison

Last edited by jeruku : 06-18-18 at 07:36 PM. Reason: Misunderstanding
  Reply With Quote
06-18-18, 12:58 PM   #9
Siku
A Fallenroot Satyr
 
Siku's Avatar
AddOn Author - Click to view addons
Join Date: Jun 2007
Posts: 29
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.
  Reply With Quote
06-18-18, 05:21 PM   #10
TOM_RUS
A Warpwood Thunder Caller
AddOn Author - Click to view addons
Join Date: Sep 2008
Posts: 95
Blizzard dev who created those API's said to not worry about memory usage.
  Reply With Quote
06-19-18, 02:19 AM   #11
SDPhantom
A Pyroguard Emberseer
 
SDPhantom's Avatar
AddOn Author - Click to view addons
Join Date: Jul 2006
Posts: 2,308
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.
__________________
WoWInterface AddOns
"All I want is a pretty girl, a decent meal, and the right to shoot lightning at fools."
-Anders (Dragon Age: Origins - Awakening)
  Reply With Quote
06-19-18, 08:17 PM   #12
thomasjohnshannon
A Theradrim Guardian
 
thomasjohnshannon's Avatar
AddOn Author - Click to view addons
Join Date: Sep 2009
Posts: 68
I never said anything but just so it clear to everyone the function that elcius posted works well.
__________________
Thomas aka Urnn
  Reply With Quote
06-21-18, 12:14 AM   #13
siweia
A Flamescale Wyrmkin
 
siweia's Avatar
AddOn Author - Click to view addons
Join Date: Jul 2011
Posts: 126
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.
  Reply With Quote
06-21-18, 12:57 AM   #14
elcius
A Cliff Giant
AddOn Author - Click to view addons
Join Date: Sep 2011
Posts: 75
Originally Posted by siweia View Post
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.
Code:
WorldMapFrame.ScrollContainer.GetCursorPosition = function(f)
	local x,y = MapCanvasScrollControllerMixin.GetCursorPosition(f);
	local s = WorldMapFrame:GetScale();
	return x/s, y/s;
end
  Reply With Quote
06-22-18, 10:59 PM   #15
siweia
A Flamescale Wyrmkin
 
siweia's Avatar
AddOn Author - Click to view addons
Join Date: Jul 2011
Posts: 126
Originally Posted by elcius View Post
Code:
WorldMapFrame.ScrollContainer.GetCursorPosition = function(f)
	local x,y = MapCanvasScrollControllerMixin.GetCursorPosition(f);
	local s = WorldMapFrame:GetScale();
	return x/s, y/s;
end
I tried the code, the memory usage went up each time you switch zones on the worldmap.
  Reply With Quote
07-02-18, 01:58 AM   #16
Nevcairiel
Premium Member
Premium Member
AddOn Author - Click to view addons
Join Date: Aug 2006
Posts: 63
Originally Posted by SDPhantom View Post
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.
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.

Last edited by Nevcairiel : 07-02-18 at 02:01 AM.
  Reply With Quote
07-07-18, 10:47 PM   #17
siweia
A Flamescale Wyrmkin
 
siweia's Avatar
AddOn Author - Click to view addons
Join Date: Jul 2011
Posts: 126
I found this in LeatixPlus by using "RunScript", and it does not produce memory waste.

Code:
local pTimer = 0

local function UpdateCharacterCoords(self, elapsed)
	pTimer = pTimer + elapsed
	if pTimer > 0.2 then
		RunScript('LeaPlusGlobalCoords = C_Map.GetPlayerMapPosition(0, "player")')
		if LeaPlusGlobalCoords then
			cChar.x:SetFormattedText("%0.1f", LeaPlusGlobalCoords.x * 100)
			cChar.y:SetFormattedText("%0.1f", LeaPlusGlobalCoords.y * 100)
		else
			cChar.x:SetText("")
			cChar.y:SetText("")
		end
		pTimer = 0
	end
end
cChar:SetScript("OnUpdate", UpdateCharacterCoords)

Last edited by siweia : 07-07-18 at 10:57 PM.
  Reply With Quote
07-07-18, 11:12 PM   #18
dssd
A Fallenroot Satyr
Join Date: May 2016
Posts: 25
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.
  Reply With Quote
07-08-18, 05:45 AM   #19
Kanegasi
A Molten Giant
 
Kanegasi's Avatar
AddOn Author - Click to view addons
Join Date: Apr 2007
Posts: 666
Originally Posted by Nevcairiel View Post
Unless you can measure an actual impact from that, its still not a problem.
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.
  Reply With Quote
07-08-18, 12:33 PM   #20
Nevcairiel
Premium Member
Premium Member
AddOn Author - Click to view addons
Join Date: Aug 2006
Posts: 63
Originally Posted by Kanegasi View Post
You can. Initiate a garbage collection manually and watch your FPS.
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.
  Reply With Quote

WoWInterface » PTR » PTR API and Graphics Changes » C_Map.GetPlayerMapPosition Memory Usage

Thread Tools
Display Modes

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

vB code is On
Smilies are On
[IMG] code is On
HTML code is Off