Thread Tools Display Modes
05-02-17, 12:14 AM   #1
gmarco
An Onyxian Warder
 
gmarco's Avatar
AddOn Author - Click to view addons
Join Date: Dec 2009
Posts: 362
Memory Leak ?

Hi all,

I was doing a simple LDB to hide the class order bar for not be shown under docking station.
But I miss something to show a panel so I make it as LDB launcher.

I thought to fill the LDB tooltip at least with a missions report so I write something like this:

Lua Code:
  1. function dataobj.OnTooltipShow(tooltip)
  2.     tooltip:AddLine(ADDON)
  3.     tooltip:AddLine(" ")
  4.     tooltip:AddLine("Missions",1,1,1,1)
  5.  
  6.         -- [ blabla ]
  7.    
  8.         local mis_avl = #C_Garrison.GetAvailableMissions(LE_FOLLOWER_TYPE_GARRISON_7_0)
  9.         local mis_pro = #C_Garrison.GetInProgressMissions(LE_FOLLOWER_TYPE_GARRISON_7_0)
  10.         local mis_fin = #C_Garrison.GetCompleteMissions(LE_FOLLOWER_TYPE_GARRISON_7_0)
  11.  
  12.         -- [ blabla ]
  13.  
  14. end

Now the problem. Is possible this make this addon MEMORY USAGE grows everytime I hover on the LDB ?




Lua Code:
  1. local mis_avl = #C_Garrison.GetAvailableMissions(LE_FOLLOWER_TYPE_GARRISON_7_0)
  2. local mis_pro = #C_Garrison.GetInProgressMissions(LE_FOLLOWER_TYPE_GARRISON_7_0)
  3. local mis_fin = #C_Garrison.GetCompleteMissions(LE_FOLLOWER_TYPE_GARRISON_7_0)
The problem are these lines... If I removed them everything is fine again.

Should I use a frame with an event like:
GARRISON_UPDATE

To calc them and not the OnTooltipShow of the LDB ?

Thanks to anyone.
__________________
This is Unix-Land. In quiet nights, you can hear the Windows machines reboot.
  Reply With Quote
05-02-17, 07:08 PM   #2
jeruku
A Cobalt Mageweaver
 
jeruku's Avatar
AddOn Author - Click to view addons
Join Date: Oct 2010
Posts: 223
Answer to your first question, it is possible. Local variables are not deleted right away, a method to get around it is to declare the function with the variables inline and simply use them from there. It, I believe, reserves the location in memory allocating it only once and will be garbage collected eventually without creating excessive copies.

Answer to your second question, yes as that is best practice. But it would also help if globals are scoped properly as it can cause some minor issues that slowly stack. This has resulted in both Blizzard developers, and addon authors, creating stack overflows.

Lua Code:
  1. local LE_FOLLOWER_TYPE_GARRISON_7_0 = LE_FOLLOWER_TYPE_GARRISON_7_0
  2. local GetAvailableMissions = C_Garrison.GetAvailableMissions
  3. local GetInProgressMissions = C_Garrison.GetInProgressMissions
  4. local GetCompleteMissions = C_Garrison.GetCompleteMissions
  5.  
  6. function dataobj.OnTooltipShow(tooltip, mis_avl, mis_pro, mis_fin)
  7.         -- [ blabla ]
  8.    
  9.         mis_avl = #GetAvailableMissions(LE_FOLLOWER_TYPE_GARRISON_7_0)
  10.         mis_pro = #GetInProgressMissions(LE_FOLLOWER_TYPE_GARRISON_7_0)
  11.         mis_fin = #GetCompleteMissions(LE_FOLLOWER_TYPE_GARRISON_7_0)
  12.  
  13.         -- [ blabla ]
  14. end
__________________
"I have not failed, I simply found 10,000 ways that did not work." - Thomas Edison
  Reply With Quote
05-03-17, 02:07 AM   #3
Kakjens
A Cliff Giant
Join Date: Apr 2017
Posts: 75
Why not declare "local mis_avl, mis_pro, mis_fin" in line 5 after "local GetCompleteMissions = C_Garrison.GetCompleteMissions", and don't call dataobj.OnTooltipShow with extra variables? Should be a bit larger average but smaller peak memory consumption.
  Reply With Quote
05-03-17, 01:41 PM   #4
gmarco
An Onyxian Warder
 
gmarco's Avatar
AddOn Author - Click to view addons
Join Date: Dec 2009
Posts: 362
... and the worst as to come

When I click to open the Draenor or Legion Hall panel I got:



Is possible that addon is charged of all the memory used by the blizzard panel ?

After a while I closed the panel I got:



So the usage is returned low.

Now I ask ... Is it a normal or expected behaviour ?

Thanks to all.
__________________
This is Unix-Land. In quiet nights, you can hear the Windows machines reboot.

Last edited by gmarco : 05-03-17 at 01:44 PM.
  Reply With Quote
05-03-17, 01:49 PM   #5
gmarco
An Onyxian Warder
 
gmarco's Avatar
AddOn Author - Click to view addons
Join Date: Dec 2009
Posts: 362
Originally Posted by Kakjens View Post
Why not declare "local mis_avl, mis_pro, mis_fin" in line 5 after "local GetCompleteMissions = C_Garrison.GetCompleteMissions", and don't call dataobj.OnTooltipShow with extra variables? Should be a bit larger average but smaller peak memory consumption.
I think because you need to get the value in the moment you make the dataobj.OnTooltipShow.

Thanks for the reply.
__________________
This is Unix-Land. In quiet nights, you can hear the Windows machines reboot.
  Reply With Quote
05-04-17, 12:55 AM   #6
SDPhantom
A Pyroguard Emberseer
 
SDPhantom's Avatar
AddOn Author - Click to view addons
Join Date: Jul 2006
Posts: 2,313
Just my two cents in on the topic. With Blizzard's recent obsession over rewriting APIs into returning tables, the inherent problem in doing so is it can easily turn into a memory hog. The two most expensive value types in Lua are functions and tables. In your function, you're requesting 3 different tables from such APIs. Even though you're only getting their length, Lua still allocated the memory to store them and they continue to wait for a garbage collection cycle to pick them up. It doesn't matter if you didn't store them in a variable, they existed for the moment they were used in an expression and therefore still take up space in memory until Lua gets rid of them later.
__________________
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 : 05-04-17 at 12:59 AM.
  Reply With Quote
05-04-17, 03:22 AM   #7
Kakjens
A Cliff Giant
Join Date: Apr 2017
Posts: 75
I meant something like
Lua Code:
  1. local LE_FOLLOWER_TYPE_GARRISON_7_0 = LE_FOLLOWER_TYPE_GARRISON_7_0
  2. local GetAvailableMissions = C_Garrison.GetAvailableMissions
  3. local GetInProgressMissions = C_Garrison.GetInProgressMissions
  4. local GetCompleteMissions = C_Garrison.GetCompleteMissions
  5. local mis_avl, mis_pro, mis_fin
  6. function dataobj.OnTooltipShow(tooltip)
  7.     -- [ blabla ]
  8.        
  9.     mis_avl = #GetAvailableMissions(LE_FOLLOWER_TYPE_GARRISON_7_0)
  10.     mis_pro = #GetInProgressMissions(LE_FOLLOWER_TYPE_GARRISON_7_0)
  11.     mis_fin = #GetCompleteMissions(LE_FOLLOWER_TYPE_GARRISON_7_0)
  12.  
  13.      -- [ blabla ]
  14. end
Also, telling that the addon in question was gmHall would have helped.

Last edited by Kakjens : 05-04-17 at 03:28 AM.
  Reply With Quote
05-04-17, 09:12 AM   #8
Lombra
A Molten Giant
 
Lombra's Avatar
AddOn Author - Click to view addons
Join Date: Nov 2006
Posts: 554
Originally Posted by Kakjens View Post
I meant something like
Lua Code:
  1. local LE_FOLLOWER_TYPE_GARRISON_7_0 = LE_FOLLOWER_TYPE_GARRISON_7_0
  2. local GetAvailableMissions = C_Garrison.GetAvailableMissions
  3. local GetInProgressMissions = C_Garrison.GetInProgressMissions
  4. local GetCompleteMissions = C_Garrison.GetCompleteMissions
  5. local mis_avl, mis_pro, mis_fin
  6. function dataobj.OnTooltipShow(tooltip)
  7.     -- [ blabla ]
  8.        
  9.     mis_avl = #GetAvailableMissions(LE_FOLLOWER_TYPE_GARRISON_7_0)
  10.     mis_pro = #GetInProgressMissions(LE_FOLLOWER_TYPE_GARRISON_7_0)
  11.     mis_fin = #GetCompleteMissions(LE_FOLLOWER_TYPE_GARRISON_7_0)
  12.  
  13.      -- [ blabla ]
  14. end
Also, telling that the addon in question was gmHall would have helped.
That does nothing.
__________________
Grab your sword and fight the Horde!
  Reply With Quote
05-04-17, 12:56 PM   #9
MunkDev
A Scalebane Royal Guard
 
MunkDev's Avatar
AddOn Author - Click to view addons
Join Date: Mar 2015
Posts: 431
Lua Code:
  1. local LE_FOLLOWER_TYPE_GARRISON_7_0 = LE_FOLLOWER_TYPE_GARRISON_7_0
  2. local GetAvailableMissions = C_Garrison.GetAvailableMissions
  3. local GetInProgressMissions = C_Garrison.GetInProgressMissions
  4. local GetCompleteMissions = C_Garrison.GetCompleteMissions
  5. local mis_avl, mis_pro, mis_fin
  6.  
  7. local frame = CreateFrame('Frame')
  8. frame:RegisterEvent('GARRISON_UPDATE')
  9. frame:SetScript('OnEvent', function()
  10.     local avl = GetAvailableMissions(LE_FOLLOWER_TYPE_GARRISON_7_0)
  11.     local pro = GetInProgressMissions(LE_FOLLOWER_TYPE_GARRISON_7_0)
  12.     local fin = GetCompleteMissions(LE_FOLLOWER_TYPE_GARRISON_7_0)
  13.  
  14.     mis_avl = avl and #avl or 0
  15.     mis_pro = pro and #pro or 0
  16.     mis_fin = fin and #fin or 0
  17. end)
  18.  
  19. function dataobj.OnTooltipShow(tooltip)
  20.     -- [ blabla ]
  21.  
  22.     -- use mis_avl, mis_pro, mis_fin here
  23.  
  24.      -- [ blabla ]
  25. end
__________________
  Reply With Quote
05-05-17, 12:41 AM   #10
gmarco
An Onyxian Warder
 
gmarco's Avatar
AddOn Author - Click to view addons
Join Date: Dec 2009
Posts: 362
Thanks to all for the kind replies.

Now I have collected all your inputs and this is the final (actual ? ) code:

Lua Code:
  1. local ADDON = ...
  2. local playerName = UnitName("player")
  3. local string_format = string.format
  4. local tooltip
  5.  
  6. local TYPE70 = LE_GARRISON_TYPE_7_0
  7. local TYPE60 = LE_GARRISON_TYPE_6_0
  8.  
  9. local FOLTYPE70 = LE_FOLLOWER_TYPE_GARRISON_7_0
  10. local FOLTYPE60 = LE_FOLLOWER_TYPE_GARRISON_6_0
  11. local FOLTYPE62 = LE_FOLLOWER_TYPE_SHIPYARD_6_2
  12.  
  13. local GetAvailableMissions = C_Garrison.GetAvailableMissions
  14. local GetInProgressMissions = C_Garrison.GetInProgressMissions
  15. local GetCompleteMissions = C_Garrison.GetCompleteMissions
  16.  
  17. local LEGION = {
  18.     avl = 0,
  19.     pro = 0,
  20.     fin = 0,
  21. }
  22.  
  23. local DRAENOR = {
  24.     avl = 0,
  25.     pro = 0,
  26.     fin = 0,
  27. }
  28.  
  29. local SHIP = {
  30.     avl = 0,
  31.     pro = 0,
  32.     fin = 0,
  33. }
  34.  
  35. local frame = CreateFrame("Frame")
  36. frame:SetScript("OnEvent", function(self, event, arg1)
  37.  
  38.     if event == "ADDON_LOADED" and arg1 == "Blizzard_OrderHallUI" then
  39.         OrderHallCommandBar:Hide()
  40.         OrderHallCommandBar.Show = function() return end
  41.         GarrisonLandingPageTutorialBox:SetClampedToScreen(true)
  42.         self:UnregisterEvent("ADDON_LOADED")
  43.     end
  44.  
  45.     if C_Garrison.GetGarrisonInfo(TYPE70) then
  46.         LEGION["avl"] = #GetAvailableMissions(FOLTYPE70)
  47.         LEGION["pro"] = #GetInProgressMissions(FOLTYPE70)
  48.         LEGION["fin"] = #GetCompleteMissions(FOLTYPE70)
  49.     end
  50.    
  51.     if C_Garrison.GetGarrisonInfo(TYPE60) then
  52.         DRAENOR["avl"] = #GetAvailableMissions(FOLTYPE60)
  53.         DRAENOR["pro"] = #GetInProgressMissions(FOLTYPE60)
  54.         DRAENOR["fin"] = #GetCompleteMissions(FOLTYPE60)           
  55.  
  56.         if C_Garrison.HasShipyard() then
  57.             SHIP["avl"] = #GetAvailableMissions(FOLTYPE62)
  58.             SHIP["pro"] = #GetInProgressMissions(FOLTYPE62)
  59.             SHIP["fin"] = #GetCompleteMissions(FOLTYPE62)                          
  60.         end    
  61.     end
  62.  
  63. end)
  64. frame:RegisterEvent("ADDON_LOADED")
  65. frame:RegisterEvent("GARRISON_UPDATE")
  66. frame:RegisterEvent("GARRISON_MISSION_LIST_UPDATE")
  67. frame:RegisterEvent("GARRISON_MISSION_FINISHED")
  68.  
  69.  
  70. local ldb = LibStub:GetLibrary("LibDataBroker-1.1")
  71. local dataobj = ldb:NewDataObject(ADDON, {
  72.     type = "data source",
  73.     icon = "Interface\\Addons\\"..ADDON.."\\icon.tga",
  74.     text = "Hall"
  75. })
  76.  
  77. dataobj.OnClick = function(self, button)  
  78.  
  79.     if InCombatLockdown() then
  80.         return
  81.     end
  82.    
  83.     if      button == "LeftButton" then
  84.         if C_Garrison.GetGarrisonInfo(TYPE70) then
  85.             if (GarrisonLandingPage and GarrisonLandingPage:IsShown()) then
  86.                 HideUIPanel(GarrisonLandingPage);
  87.             else
  88.                 ShowGarrisonLandingPage(TYPE70);
  89.             end
  90.         end
  91.     elseif  button == "RightButton" then   
  92.         if C_Garrison.GetGarrisonInfo(TYPE60) then
  93.             if (GarrisonLandingPage and GarrisonLandingPage:IsShown()) then
  94.                 HideUIPanel(GarrisonLandingPage);
  95.             else
  96.                 ShowGarrisonLandingPage(TYPE60);
  97.             end
  98.         end
  99.     elseif  button == "MiddleButton" then
  100.     end
  101.    
  102. end
  103.  
  104. function dataobj.OnTooltipShow(tooltip)
  105.     tooltip:AddLine(ADDON)
  106.     tooltip:AddLine(" ")
  107.     tooltip:AddLine("Missions",1,1,1,1)
  108.  
  109.     if C_Garrison.GetGarrisonInfo(TYPE70) then
  110.        
  111.         tooltip:AddDoubleLine("Legion Order Hall",
  112.                 string_format("|cFFFFFFFF%d|r", LEGION["avl"]) .. " "  ..
  113.                 string_format("|cFFFFFF00%d|r", LEGION["pro"] - LEGION["fin"]) .. " " ..
  114.                 string_format("|cFF00803F%d|r", LEGION["fin"])
  115.         )
  116.  
  117.     end
  118.    
  119.     if C_Garrison.GetGarrisonInfo(TYPE60) then
  120.  
  121.         tooltip:AddDoubleLine("Draenor Garrison",
  122.                 string_format("|cFFFFFFFF%d|r", DRAENOR["avl"]) .. " "  ..
  123.                 string_format("|cFFFFFF00%d|r", DRAENOR["pro"] - DRAENOR["fin"]) .. " " ..
  124.                 string_format("|cFF00803F%d|r", DRAENOR["fin"])
  125.         )
  126.  
  127.         if C_Garrison.HasShipyard() then
  128.             tooltip:AddDoubleLine("Draenor Shipyard",
  129.                     string_format("|cFFFFFFFF%d|r", SHIP["avl"]) .. " "  ..
  130.                     string_format("|cFFFFFF00%d|r", SHIP["pro"] - SHIP["fin"]) .. " " ..
  131.                     string_format("|cFF00803F%d|r", SHIP["fin"])
  132.             )
  133.         end
  134.        
  135.     else
  136.    
  137.         tooltip:AddLine("No Hall or Garrison yet",1,1,0,0)
  138.    
  139.     end
  140.    
  141.     tooltip:AddLine(" ")
  142.     tooltip:AddDoubleLine("Left Click",     "Class Hall")
  143.     tooltip:AddDoubleLine("Right Click",    "Garrison")
  144. end

Using a frame to collect data basing on events surely helped in decreasing the memory allocated ... even if it grows a lot the same when you click to get the panels but I think it can't be avoided

Thanks again.
__________________
This is Unix-Land. In quiet nights, you can hear the Windows machines reboot.
  Reply With Quote
05-05-17, 01:58 AM   #11
Fizzlemizz
I did that?
 
Fizzlemizz's Avatar
Premium Member
AddOn Author - Click to view addons
Join Date: Dec 2011
Posts: 1,871
Using table[somevar] = "something" is great for accessing unknown table keys. If you know the key you want to use you can access it with dot notation table.key = "something"

Example in your case:
Lua Code:
  1. if C_Garrison.GetGarrisonInfo(TYPE70) then
  2.         LEGION["avl"] = #GetAvailableMissions(FOLTYPE70)
  3.         LEGION["pro"] = #GetInProgressMissions(FOLTYPE70)
  4.         LEGION["fin"] = #GetCompleteMissions(FOLTYPE70)
  5. end
could be:
Lua Code:
  1. if C_Garrison.GetGarrisonInfo(TYPE70) then
  2.         LEGION.avl = #GetAvailableMissions(FOLTYPE70)
  3.         LEGION.pro = #GetInProgressMissions(FOLTYPE70)
  4.         LEGION.fin = #GetCompleteMissions(FOLTYPE70)
  5. end

Just an FYI.
Then again, I dislike wasting keystrokes like using brackets around if statements or ending lines with semicolons etc. .
__________________
Fizzlemizz
Maintainer of Discord Unit Frames and Discord Art.
Author of FauxMazzle, FauxMazzleHUD and Move Pad Plus.

Last edited by Fizzlemizz : 05-05-17 at 02:35 AM.
  Reply With Quote

WoWInterface » Developer Discussions » Lua/XML Help » Memory Leak ?

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