Thread Tools Display Modes
06-02-19, 07:18 PM   #1
galvin
A Frostmaul Preserver
AddOn Author - Click to view addons
Join Date: Nov 2006
Posts: 265
How do I make this options list faster?

To repeat this just download GalvinUnitBars from curse:
https://wow.curseforge.com/projects/.../files/2710572
Then go to general -> Alt Power Bar
Make sure "Show used bars only" is unchecked

The problem is each time I leave this options window and go back. The same lag happens.
I'd like to know how I can speed this up.

Code:
local function BuildAltPowerBarList(APA, Order, Name, TableName)
  local APBUsed = Main.APBUsed
  local APBUseBlizz = Main.APBUseBlizz

  local PowerBarList = {
    type = 'group',
    name = Name,
    order = Order,
    args = {},
    disabled = function()
                 return Main.UnitBars.APBDisabled
               end,
  }
  APA[TableName] = PowerBarList
  local PBA = PowerBarList.args

  for BarID = 1, 10000 do
    local AltPowerType, MinPower, _, _, _, _, _, _, _, _, PowerName, PowerTooltip = GetAlternatePowerInfoByID(BarID)
    local ZoneName = APBUsed[BarID]

    if TableName ~= 'APB' then
      if AltPowerType and APBUseBlizz[BarID] then
        local Index = 'APBUseBlizz' .. BarID
        PBA[Index] = APA.APB.args[Index]
      end
    elseif AltPowerType then
      PBA['APBUseBlizz' .. BarID] = {
        type = 'toggle',
        width = 'full',
        arg = BarID,
        name = function()
                 if ZoneName then
                   return format('|cff00ff00%s|r : |cffffff00%s|r (|cff00ffff%s|r)', BarID, PowerName, ZoneName)
                 else
                   return format('|cff00ff00%s|r : |cffffff00%s|r', BarID, PowerName)
                 end
               end,
        order = function()
                  if APBUsed[BarID] then
                    return BarID
                  else
                    return BarID + 1000
                  end
                end,
    --[[    hidden = function()
                   MyCount = MyCount + 1
                   print(MyCount)
                   local APBShowUsed = Main.UnitBars.APBShowUsed
                   if AltPowerBarSearch == '' or BarID == tonumber(AltPowerBarSearch) or
                                                 strfind(strlower(PowerName),    strlower(AltPowerBarSearch)) or
                                                 strfind(strlower(PowerTooltip), strlower(AltPowerBarSearch)) or
                                                 ZoneName and strfind(strlower(ZoneName), strlower(AltPowerBarSearch)) then
                     if APBShowUsed and APBUsed[BarID] ~= nil then
                       return false
                     elseif not APBShowUsed then
                       return false
                     end
                   end
                   return true
                 end, --]]
        disabled = function()
                     return Main.HasAltPower
                   end,
      }
      PBA['Line2' .. BarID] = {
        type = 'description',
        name = PowerTooltip,
        fontSize = 'medium',
        order = function()
                  if APBUsed[BarID] then
                    return BarID + 0.2
                  else
                    return BarID + 1000.3
                  end
                end,
        hidden = function()
                   return APBUsed[BarID] == nil and Main.UnitBars.APBShowUsed
                 end,
      }
    end
  end
end

local function CreateAltPowerBarOptions(Order, Name)
  local APA = nil

  local AltPowerBarOptions = {
    type = 'group',
    name = Name,
    order = Order,
    childGroups = 'tab',
    get = function(Info)
            local KeyName = Info[#Info]

            if strfind(KeyName, 'APBUseBlizz') then
              return Main.APBUseBlizz[Info.arg]
            elseif KeyName == 'Search' then
              return AltPowerBarSearch
            else
              return Main.UnitBars[KeyName]
            end
          end,
    set = function(Info, Value)
            local KeyName = Info[#Info]

            if strfind(KeyName, 'APBUseBlizz') then
              Main.APBUseBlizz[Info.arg] = Value
            elseif KeyName == 'Search' then
              AltPowerBarSearch = Value
           --   BuildAltPowerBarList(APA, 100, 'Alternate Power Bar', 'APB')
           --   BuildAltPowerBarList(APA, 101, 'Use Blizzard', 'APBUseBlizz')
            else
              if APA and strfind(KeyName, 'APBDisabled') and Value then
                APA.PowerBarList = nil
              end
              Main.UnitBars[KeyName] = Value
            end
          end,
    disabled = function()
                 if not Main.UnitBars.APBDisabled then
           --        BuildAltPowerBarList(APA, 100, 'Alternate Power Bar', 'APB')
           --        BuildAltPowerBarList(APA, 101, 'Use Blizzard', 'APBUseBlizz')
                 end
                 return Main.HasAltPower
               end,
    args = {
      Description = {
        type = 'description',
        name = 'May take a few seconds to build the list. Bars already used have an area name.\nChecking off a bar will use the blizzard bar instead',
        order = 1,
      },
      Search = {
        type = 'input',
        name = 'Search',
        order = 3,
        disabled = function()
                     return Main.UnitBars.APBDisabled or Main.HasAltPower
                   end,
      },
      clearSearch = {
        type = 'execute',
        name = 'Clear',
        desc = 'Clear search',
        width = 'half',
        order = 4,
        func = function()
                 AltPowerBarSearch = ''
                 BuildAltPowerBarList(APA, 100, 'Alternate Power Bar', 'APB')
                 BuildAltPowerBarList(APA, 101, 'Use Blizzard', 'APBUseBlizz')
                 HideTooltip(true)
               end,
        disabled = function()
                     return Main.UnitBars.APBDisabled or Main.HasAltPower
                   end,
      },
      APBShowUsed = {
        type = 'toggle',
        name = 'Show used bars only',
        width = 'normal',
        order = 5,
        disabled = function()
                     return Main.UnitBars.APBDisabled or Main.HasAltPower
                   end,
      },
      APBDisabled = {
        type = 'toggle',
        name = 'Disable',
        width = 'half',
        order = 6,
      },
      Spacer10 = CreateSpacer(10),
    },
  }

  APA = AltPowerBarOptions.args
  BuildAltPowerBarList(APA, 100, 'Alternate Power Bar', 'APB')
  BuildAltPowerBarList(APA, 101, 'Use Blizzard', 'APBUseBlizz')

  return AltPowerBarOptions
end
  Reply With Quote
06-02-19, 07:44 PM   #2
Kanegasi
A Molten Giant
 
Kanegasi's Avatar
AddOn Author - Click to view addons
Join Date: Apr 2007
Posts: 666
Lua Code:
  1. for BarID = 1, 10000 do

This is most likely the source of the lag and completely unnecessary. I’m assuming the bar IDs include any displayed bars for raid members. This means that the maximum amount of available alt power bars at once should be 80, assuming a complicated encounter with two bars and a full raid of 40. Realistically, the maximum is 50. If you want to play it safe, reduce that 10000 to 99.
  Reply With Quote
06-02-19, 08:12 PM   #3
galvin
A Frostmaul Preserver
AddOn Author - Click to view addons
Join Date: Nov 2006
Posts: 265
Its not for a raid. Its an options list to pick which alternate powerbars you want to use. If the user checks one off. It uses blizzards own artwork instead. Otherwise it uses a statusbar to show it.

Its not creating 10000 objects. So that part isn't effecting anything. Its creating as many powerbars as it finds.
  Reply With Quote
06-02-19, 09:36 PM   #4
Terenna
A Flamescale Wyrmkin
AddOn Author - Click to view addons
Join Date: Jun 2016
Posts: 105
That code is literally doing 10,000 function calls, 10,000-40,000 concatenates. Nearly 100,000 if checks and 100,000 table look ups. It doesn't matter how many options they've set, there's no check for that. As written, that code does that for loop 10,000 times. The very first line alone within it will cause 10,000 function runs via GetAlternateInfoByID()

I'd bet a lot of money that's where your lag is coming from
  Reply With Quote
06-02-19, 10:16 PM   #5
galvin
A Frostmaul Preserver
AddOn Author - Click to view addons
Join Date: Nov 2006
Posts: 265
No, you underestimate how fast lua runs. I'd have to set it to a million something before you'd notice lag.
I did change it to a 1000, but it didn't change anything in terms of lag.

What's causing the lag is building the objects. On PTR this is even worse, it lags around 10x

It's not the problem
  Reply With Quote
06-03-19, 11:55 AM   #6
JDoubleU00
A Firelord
 
JDoubleU00's Avatar
AddOn Author - Click to view addons
Join Date: Mar 2008
Posts: 463
Originally Posted by galvin View Post
No, you underestimate how fast lua runs. I'd have to set it to a million something before you'd notice lag.
I did change it to a 1000, but it didn't change anything in terms of lag.

What's causing the lag is building the objects. On PTR this is even worse, it lags around 10x

It's not the problem
Did you actually try changing the value to determine if it is or isn't the problem?
__________________
Author of JWExpBar and JWRepBar.
  Reply With Quote
06-03-19, 01:29 PM   #7
jeruku
A Cobalt Mageweaver
 
jeruku's Avatar
AddOn Author - Click to view addons
Join Date: Oct 2010
Posts: 223
Use what SDPhantom said to dissuade everyone.

Personally I'd use more tables than concatenations since memory references take both CPU cycles and RAM, it can actually take less time just to use an extra CPU cycle.
Lua Code:
  1. PBA.APBUseBlizz[BarID] -- two simple table lookups
  2. --instead of PBA['APBUseBlizz' .. BarID] -- You create two strings in RAM, access both, and do a table lookup


Scope helps, creating new locals every single time in a loop creates them that many times. And garbage collection isn't guaranteed to work immediately after the function is resolved.
Lua Code:
  1. local AltPowerType, MinPower, PowerName, PowerTooltip, ZoneName, _ -- _ is still a variable and is a common global leak in addons
  2. for BarID = 1, 10000 do
  3.     AltPowerType, MinPower, _, _, _, _, _, _, _, _, PowerName, PowerTooltip = GetAlternatePowerInfoByID(BarID)
  4.     ZoneName = APBUsed[BarID]


Making If statements with a common denominator helps
Lua Code:
  1. local Index
  2. --scoping mentioned above
  3. if AltPowerType then
  4.     if TableName ~= 'APB'  and APBUseBlizz[BarID] then
  5.         Index = APBUseBlizz[BarID]
  6.     else
  7.         PBA.APBUseBlizz[BarID] = {}
  8.     end
  9. end
__________________
"I have not failed, I simply found 10,000 ways that did not work." - Thomas Edison

Last edited by jeruku : 06-03-19 at 05:47 PM. Reason: Thanks, SDPhantom, I learned something new.
  Reply With Quote
06-03-19, 04:31 PM   #8
SDPhantom
A Pyroguard Emberseer
 
SDPhantom's Avatar
AddOn Author - Click to view addons
Join Date: Jul 2006
Posts: 2,313
Originally Posted by jeruku View Post
Use GetTime() before and after the call/loop to dissuade everyone.
Unless they changed it again, GetTime() only updates its value when a new frame is drawn. You should use debugprofilestart() and debugprofilestop(). It's debatable whether the former is even necessary as you could just compare the timestamps from the later. Also other addons could be using this to precisely keep track of time. Lua is a single-thread environment, but keep this in mind when calling functions from other addons.
__________________
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-03-19 at 04:44 PM.
  Reply With Quote
06-03-19, 06:27 PM   #9
galvin
A Frostmaul Preserver
AddOn Author - Click to view addons
Join Date: Nov 2006
Posts: 265
The overhead of creating objects in ace3 is pretty high on the PTR. On live it creates some lag but only 1.5seconds. And since its out of combat stuff. Don't think people will mind it much.

Yeah I already cut the loop down to 1000 iterations. There's only around 200 alternate power bars total.

Underscores are already localized. Everything is localized if possible.
The actually amount of work going on is around 200 bars. That's not much work at all. The lag is
coming from making the objects in ace3.

Here a video showing what I'm doing. All this stuff works on live. But on PTR those lag spikes are 15 to 20seconds long.

https://www.youtube.com/watch?v=RZ1f2QBqojk
  Reply With Quote
06-04-19, 12:54 AM   #10
galvin
A Frostmaul Preserver
AddOn Author - Click to view addons
Join Date: Nov 2006
Posts: 265
Update the code, still runs dog slow on 8.2 PTR, takes around 1 second on live and 15+ on PTR. Fast as I can make it

Code:
local function BuildAltPowerBarList(APA, Order, Name, TableName)
  local APBUsed = Main.APBUsed
  local APBUseBlizz = Main.APBUseBlizz

  local PowerBarList = {
    type = 'group',
    name = Name,
    order = Order,
    args = {},
    disabled = function()
                 return Main.UnitBars.APBDisabled
               end,
  }
  APA[TableName] = PowerBarList
  local PBA = PowerBarList.args

  for BarID = 1, 1000 do
    local AltPowerType, MinPower, _, _, _, _, _, _, _, _, PowerName, PowerTooltip = GetAlternatePowerInfoByID(BarID)
    local ZoneName = APBUsed[BarID]

    if TableName ~= 'APB' then
      if AltPowerType and APBUseBlizz[BarID] then
        local Index = 'APBUseBlizz' .. BarID
        PBA[Index] = APA.APB.args[Index]
      end
    elseif AltPowerType then
      PBA['APBUseBlizz' .. BarID] = {
        type = 'toggle',
        width = 'full',
        arg = BarID,
        name = function()
                 if ZoneName then
                   return format('|cff00ff00%s|r : |cffffff00%s|r (|cff00ffff%s|r)', BarID, PowerName, ZoneName)
                 else
                   return format('|cff00ff00%s|r : |cffffff00%s|r', BarID, PowerName)
                 end
               end,
        order = function()
                  if APBUsed[BarID] then
                    return BarID
                  else
                    return BarID + 1000
                  end
                end,
        hidden = function()
                   local APBShowUsed = Main.UnitBars.APBShowUsed
                   if AltPowerBarSearch == '' or BarID == tonumber(AltPowerBarSearch) or
                                                 strfind(strlower(PowerName),    strlower(AltPowerBarSearch)) or
                                                 strfind(strlower(PowerTooltip), strlower(AltPowerBarSearch)) or
                                                 ZoneName and strfind(strlower(ZoneName), strlower(AltPowerBarSearch)) then
                     if APBShowUsed and APBUsed[BarID] ~= nil then
                       return false
                     elseif not APBShowUsed then
                       return false
                     end
                   end
                   return true
                 end,
      }
      PBA['Line2' .. BarID] = {
        type = 'description',
        name = PowerTooltip,
        fontSize = 'medium',
        order = function()
                  if APBUsed[BarID] then
                    return BarID + 0.2
                  else
                    return BarID + 1000.3
                  end
                end,
        hidden = PBA['APBUseBlizz' .. BarID].hidden,
      }
    end
  end
end

local function CreateAltPowerBarOptions(Order, Name)
  local APA = nil

  local AltPowerBarOptions = {
    type = 'group',
    name = Name,
    order = Order,
    childGroups = 'tab',
    get = function(Info)
            local KeyName = Info[#Info]

            if strfind(KeyName, 'APBUseBlizz') then
              return Main.APBUseBlizz[Info.arg]
            elseif KeyName == 'Search' then
              return AltPowerBarSearch
            else
              return Main.UnitBars[KeyName]
            end
          end,
    set = function(Info, Value)
            local KeyName = Info[#Info]

            if strfind(KeyName, 'APBUseBlizz') then
              Main.APBUseBlizz[Info.arg] = Value
            elseif KeyName == 'Search' then
              AltPowerBarSearch = Value
            else
              Main.UnitBars[KeyName] = Value
            end
          end,
    disabled = function()
                 return Main.HasAltPower
               end,
    args = {
      Description = {
        type = 'description',
        name = 'May take a few seconds to build the list. Bars already used have an area name.\nChecking off a bar will use the blizzard bar instead',
        order = 1,
      },
      Search = {
        type = 'input',
        name = 'Search',
        order = 3,
        disabled = function()
                     return Main.UnitBars.APBDisabled or Main.HasAltPower
                   end,
      },
      clearSearch = {
        type = 'execute',
        name = 'Clear',
        desc = 'Clear search',
        width = 'half',
        order = 4,
        func = function()
                 AltPowerBarSearch = ''
                 HideTooltip(true)
               end,
        disabled = function()
                     return Main.UnitBars.APBDisabled or Main.HasAltPower
                   end,
      },
      APBShowUsed = {
        type = 'toggle',
        name = 'Show used bars only',
        width = 'normal',
        order = 5,
        disabled = function()
                     return Main.UnitBars.APBDisabled or Main.HasAltPower
                   end,
      },
      APBDisabled = {
        type = 'toggle',
        name = 'Disable',
        width = 'half',
        order = 6,
      },
      Spacer10 = CreateSpacer(10),
    },
  }

  APA = AltPowerBarOptions.args
  BuildAltPowerBarList(APA, 100, 'Alternate Power Bar', 'APB')
  BuildAltPowerBarList(APA, 101, 'Use Blizzard', 'APBUseBlizz')

  return AltPowerBarOptions
end

Last edited by galvin : 06-04-19 at 01:00 AM.
  Reply With Quote

WoWInterface » Developer Discussions » Lua/XML Help » How do I make this options list faster?

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