Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#458 closed defect (fixed)

huge pause +memleak when selecting lots of different unit types

Reported by: beherith Owned by: Bluestone
Priority: major Milestone:
Component: Chili Version:
Keywords: Cc:

Description (last modified by Bluestone)


Change History (9)

comment:1 by beherith, 7 years ago

[f=0017347] Error: assert(CTO >= LTO) failed (SF=17347 : DF=49643 : CTO=0.000000 : LTO=0.240000 : WSF=0.030000 : DT=0.000000ms) [f=0017413] Error: assert(CTO <= 1.0f) failed (SF=17413 : DF=49687 : CTO=1.537255 : WSF=0.048039 : DT=32.000000ms : NP=0) [f=0017429] Error: assert(CTO <= 1.0f) failed (SF=17429 : DF=49707 : CTO=1.345098 : WSF=0.048039 : DT=28.000000ms : NP=1) [f=0018213] Error: assert(CTO <= 1.0f) failed (SF=18213 : DF=50522 : CTO=1.400000 : WSF=0.040000 : DT=35.000000ms : NP=0)

related?

comment:2 by beherith, 7 years ago

Also IMPORTANT: hit debug view, and /give all. Select and deselect all repeatedly, it will cause a lua memory leak. You can see lua mem usage in bottom left corner of debug view.

comment:3 by Bluestone, 7 years ago

Owner: set to Bluestone

There is certainly something major wrong but those failed asserts are about slightly delays in synced-unsynced timekeeping (largest is 35ms) and I'm pretty sure they aren't related.

comment:4 by Bluestone, 7 years ago

The memory leak is caused by funks_smenu - it's a bug in either that or chili

N2S: I suspect this is related to loading each panel 'from new' every time - on each menu change all children are cleared and then reloaded as new objects. Should load all the picture panels at start and show/move/hide only one always existing panel for each unitpic.

comment:5 by Bluestone, 7 years ago

Description: modified (diff)
Summary: Chili causes a huge pause when selecting lots of different unit typeshuge pause +memleak when selecting lots of different unit types

comment:6 by Bluestone, 7 years ago

further N2S: queue visuals on smenu can be cleared & reloaded each time without significant overhead, so set these up as children of the picture panels.

comment:7 by Jalmari Ikävalko, 7 years ago

I am completely unsure if it's relevant, but if you uncomment "Spring.Echo(("Chili: tried remove none child \"%s\" from \"%s\"!"):format(child.name, self.name))" from chili/object.lua's RemoveChild()-function, it becomes apparent that Chili tries to free a bunch of children when selecting an unit but can not find the given children.

The easiest fix does seem to be simply reusing images and buttons for menus.

comment:8 by Jalmari Ikävalko, 7 years ago

The memory leak can be hacked out by commenting out the following line:

if buildMenu.choice ~= i then buildGrids[i]:Hide() end

After noticing that, I did the following changes to luaui/gui_chili_funks_sMenu.lua, which fixes the memleak without commenting out the Hide() call:

--------------------------- 
-- Loads/reloads the icon panels for commands
local function loadPanels()
	orderMenu:ClearChildren()
	stateMenu:ClearChildren()

	for i=1,#catNames do
		if buildGrids[i] then
			buildGrids[i]:Show() -- non visible objects do not succesfully get cleared?
			--buildGrids[i]:ClearChildren() -- doesn't seem necessary?
		end
		--buildGrids[i] = {} -- doesn't seem necessary?
	end

	for i=1,#catNames do
		buildArray[i] = {}
	end
	
	buildMenu:ClearChildren()

	orderArray = {}
	stateArray = {}
	
	local units = spGetSelectedUnits()
	if units[1] and (selectedUnits[1] ~= units[1]) then
		selectedUnits = units
		updateTab = true
	end
	createMenus()
	makeMenuTabs()
end

It seems that the problem was mainly because hidden children do not get properly cleared. For Chili's RemoveChild() to success, Show() must be a succes, too. However, if an object has no parent, Show() will fail and as such RemoveChild() will fail. There was no parent because buildMenu:ClearChildren() cleared the parents of buildGrid. As such, any calls to RemoveChild() with buildGrids' member as an argument, failed.

..At least I think so. *whistles*

Also:

[18:45:54] <[Fx]Bluestone> oh, i was suggesting to make the entries of buildgrid all children of buildmenu so as they got cleared in the already present clearchildren

I tried setting buildGrids entries to have buildMenu as their parent, but the memory leak persisted without adding the above fix. :-/ Could be fixable in Chili's side, tho. It seems to not handle making children of children visible before deleting children.

comment:9 by Bluestone, 7 years ago

Resolution: fixed
Status: newclosed

[17:19:53] <[Fx]Bluestone> the bug was that apparently images have to be shown before they can be correctly cleared [17:20:09] <[Fx]Bluestone> so it was a chili bug! [17:20:19] <[Fx]Bluestone> i'll make some cute little demo and pass it onto jK

Last edited 7 years ago by Bluestone (previous) (diff)
Note: See TracTickets for help on using tickets.