WoWInterface

WoWInterface (https://www.wowinterface.com/forums/index.php)
-   Lua/XML Help (https://www.wowinterface.com/forums/forumdisplay.php?f=16)
-   -   Memory Leak ? (https://www.wowinterface.com/forums/showthread.php?t=55369)

gmarco 05-02-17 12:14 AM

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.

jeruku 05-02-17 07:08 PM

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

Kakjens 05-03-17 02:07 AM

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.

gmarco 05-03-17 01:41 PM

... 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.

gmarco 05-03-17 01:49 PM

Quote:

Originally Posted by Kakjens (Post 323203)
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.

SDPhantom 05-04-17 12:55 AM

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.

Kakjens 05-04-17 03:22 AM

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.

Lombra 05-04-17 09:12 AM

Quote:

Originally Posted by Kakjens (Post 323215)
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.

MunkDev 05-04-17 12:56 PM

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

gmarco 05-05-17 12:41 AM

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.

Fizzlemizz 05-05-17 01:58 AM

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. ;).


All times are GMT -6. The time now is 02:59 PM.

vBulletin © 2024, Jelsoft Enterprises Ltd
© 2004 - 2022 MMOUI