Huge tooltip is extremely slow #194


  • Defect
  • Fixed
Closed
Assigned to allarastarmist
  • Urtgard created this issue Jul 14, 2016
    Contributor

    I played a bit on the beta server with a complete new char. No mounts, no pets and no toys.
    I looked a the rarity tooltip and it was huge (Who would have thought that? :D).
    To get a better overview I tried to collapse the mount section. The game freezes for more than 10 seconds.
    Next I wanted to collapse the pet section. The game freezes again and after some time I got kicked off the server.

    So a huge tooltip is not only huge it's slow too.
    I've never noticed a complaint due to this. So I guess most people collected enough that the tooltip is fast enough or a new problem with 7.0 (can't test 6.2).

  • Urtgard added the tags New Defect Jul 14, 2016
  • Urtgard removed a tag Enhancement Jul 14, 2016
  • Urtgard added a tag Defect Jul 14, 2016
  • AllaraStarmist posted a comment Jul 14, 2016

    Yeah I had the same problem. I actually think it's an issue with the beta servers and item requests. My theory is this won't happen on live. I tested a pretty damn big tooltip on live and it works fine.

  • AllaraStarmist removed a tag New Jul 14, 2016
  • AllaraStarmist added a tag Waiting Jul 14, 2016
  • AllaraStarmist posted a comment Jul 20, 2016

    Still slow, but not locking up the client anymore, at least for me. Closing this unless something else crops up.

  • AllaraStarmist removed a tag Waiting Jul 20, 2016
  • AllaraStarmist added a tag Replied Jul 20, 2016
  • AllaraStarmist removed a tag Replied Jul 20, 2016
  • AllaraStarmist added a tag Invalid Jul 20, 2016
  • AllaraStarmist closed issue Jul 20, 2016
  • AllaraStarmist removed a tag Invalid Jul 25, 2016
  • AllaraStarmist added a tag Accepted Jul 25, 2016
  • AllaraStarmist reopened issue Jul 25, 2016
  • AllaraStarmist posted a comment Jul 25, 2016

    This is an issue on live. Need to conduct profiling on the tooltip renderer to see if I can actually find the source of the problem. Beyond that, I also want to try implementing multiple LDB feeds to spread this out better.

  • AllaraStarmist removed a tag Accepted Jul 25, 2016
  • AllaraStarmist added a tag New Jul 25, 2016
  • AllaraStarmist removed a tag New Jul 25, 2016
  • AllaraStarmist added a tag Accepted Jul 25, 2016
  • AllaraStarmist posted a comment Jul 25, 2016

    OK I think I traced this down to the tooltip library itself. Something about reconfiguring hundreds of frames at once is now prohibitively slow in WoW 7.0. To fix this, I'm going to have to disable collapsing on the group headers. I'm also going to add several new LDB feeds to Rarity, each of which show only one type of item. This way users can look at smaller tooltips if they want to.

  • Urtgard posted a comment Jul 25, 2016

    I've read about a problem with GetItemInfo and caching the data you get. Have you tried a huge tooltip with static data? Without GetItemInfo?

  • AllaraStarmist posted a comment Jul 25, 2016

    Do you have a link to that? That used to be an issue years ago, but since I think Cataclysm it hasn't been needed.

    I'm doing profiling of the code and I tried running the entire tooltip renderer EXCEPT actually adding the row (which LibQTip handles). The tooltip renders instantly. This test included all the GetItemInfo calls and everything.

    My assumption so far is it's the tooltip itself which is the problem. I'm exploring if there is any way to change my use of the library to get around this.

    Right now it looks like I'll have to disable collapsing the groups and instead put toggles into Options to turn the groups off.

  • AllaraStarmist posted a comment Jul 25, 2016

    Aha, I think I fixed it. Completely throwing away (releasing) the tooltip during an expand/collapse and rebuilding it from scratch seems to eliminate the issue.

    I have one player saying just hovering over the feed/icon causes the hang/crash, but I've never seen that behavior and can't reproduce, and it is not consistent with what I believe the issue is. Only the collapse/expand operation was causing the hang, and I think it was due to recursive relayout behavior inside LibQTip, which is solved by throwing out the tooltip entirely and starting over. I consider this a design error in LibQTip, but I can't go in there changing things.

    The highest I can get now is 78ms of render time with everything expanded.

    Going to push this out and leave the ticket open and wait to see if it clears it up.

  • AllaraStarmist posted a comment Jul 25, 2016

    I hope that's not really the issue, and if it is I hope Blizzard fixes it. Refactoring Rarity to handle unreliable item cache will be pretty hard. The client has handled rapid GetItemInfo calls for years now, which Blizzard intentionally made to work when it didn't used to. Them breaking that is a major step back.

  • AllaraStarmist posted a comment Jul 27, 2016

    I'm implementing a local item cache in Rarity with the next alpha, which progressively pulls down all 556 items in advance over about 5.5 seconds when your UI first loads. It's still doing 100 items a second (10 every tenth of a second), but this is a lot less load on the Blizzard API than hitting it hundreds of times in a single frame. Once primed, Rarity relies solely on its own local in-memory cache for item info. I wasn't seeing any problems, but as Rarity continues to scale I figure this is a good step to take.

  • Urtgard posted a comment Jul 28, 2016

    Why don't you cache only items you actually need? Totally working code :D

    function GetItemInfoCache(itemID)
       if item is cached return local iteminfo
      else call GetItemInfo(itemID), put data in cache and return iteminfo
    end
    

    In practice you'll only need all items if you load the options. And this doesn't happen very often. So why would you cache all items?

    Or am I missing something here?

  • AllaraStarmist posted a comment Jul 28, 2016

    I already implemented the cache, and it's a bit more complex than your code, but thanks. Blizzard's API can return nil, at which point we must throw it out. It's also a table that needs to be unpacked, etc. See my implementation for the correct approach.

    You answered the "why" yourself: if you need to open Options. Might as well cache everything. I also need to cache a bunch of stuff for tooltip additions anyway—FAR more than just the tracked items—so might as well prime it with everything. It only takes 5.6 seconds right now. I could ramp that up, but I think it's fine.

    Keep in mind that priming the cache is strictly optional in terms of the add-on operating. If for some reason the player does something during those 5.6 seconds (such as obtain a new item or make an attempt), the cache will preempt the priming and just "work". My architecture allows for the add-on to not even realize there is a cache layer. The entire goal here is to prevent hitting the API for 500 items in one frame, which this achieves.

    There's actually a flaw in the current implementation that I will be fixing soon. Since the game can rarely return nil on some logins, I have to continually retry cache priming until I get them all. I'll do subsequent priming in the background without the "Loading" message and without blocking the tooltip.


    Edited Jul 28, 2016
  • AllaraStarmist posted a comment Jul 28, 2016

    I think I finally realized where you were being naive. Basically your implementation, without priming, still results in up to hundreds (could just be several dozen if you have a lot of items) of calls hitting Blizzard in one frame (when the tooltip is generated). The problem is I have users complaining that the item cache is unreliably slow at times. I'm not convinced any of this is even necessary, but it was a fun afternoon exercise and doesn't cause any problems. Once Rarity has primed itself (again, the goal here is to prime it *slowly*, not all in one frame as it currently does), it no longer relies on the Blizzard API at all. This can only be an improvement, especially since they have demonstrated a knack for breaking their own shit in patches now.

  • Urtgard posted a comment Jul 28, 2016

    Thanks for the detailed explanation.
    Of course my code snippet is missing a lot. Was just there to clarify what I meant.

    Even the huge tooltip on my beta account works flawlessly :)

    Just two notes 1st world problems:

    • Can you add an option to deactivate/change the delay for the tooltip you added in r533? I find it feels clunky with the delay.
    • For the first few seconds after a reload/login you can't access the tooltip. Maybe add a little placeholder tooltip like "Tooltip is loading."

    Anyway good job and keep it going :)

  • AllaraStarmist posted a comment Jul 28, 2016

    Will do both!

  • AllaraStarmist posted a comment Jul 29, 2016

    After extensive testing I was able to locate a race condition that causes reentrance on the tooltip renderer, which causes it to render very slowly whilst hanging the client. The reentrance is caused because Rarity issues one ShowTooltip call on a timer a few seconds after the item cache is done populating. This call is issued in order to provide the holiday reminders. If the player tries to show the tooltip at the same exact time as the timer fires, it hangs the client. On my system with relatively fewer items, the hang lasts 3-5 seconds. If the delay is exponential, as I expect it is, users with more items than me could be seeing drastically higher times. Depending on the nature of the underlying issue, memory growth could also be resulting in a client crash.

    All of this is solved by making ShowTooltip non-reentrant, which I did in r542. I have asked a user on Curse to test it.

  • AllaraStarmist posted a comment Jul 29, 2016

    So that was one source of crashing, but there was yet another one that happened if the hide delay was > 0 and the user moused into the display while the tooltip was already showing. That was fixed in r543.

  • AllaraStarmist posted a comment Jul 30, 2016

    Think we finally nailed this. Closing.

  • AllaraStarmist removed a tag Accepted Jul 30, 2016
  • AllaraStarmist added a tag Fixed Jul 30, 2016
  • AllaraStarmist closed issue Jul 30, 2016

To post a comment, please login or register a new account.