Destroy() does not decrease memory usage?

ScribeOfTheEndless · October 10, 2019, 8:49pm

Hello,

I’m trying to decrease the amount of memory being used in my game. I’ve reduced the wasted scripts, dramatically reduced the amount of parts being used, and came to what I hope is the final area to condense: GUIs.

I’ve reduced my actively running GUIs down to one, a single Menu Management one, player clicks the button, it opens a frame with clickable buttons to clone that Gui from the ReplicatedStorage to the PlayerGui. This will of course add more memory being used, sometimes dramatically depending on the size of the Gui being cloned. Then when the player closes out the Gui, the Gui is set to destroy() .

The Gui is destroyed from the PlayerGui, but the memory usage does not decrease. I assume the Guis are somehow being cached? And as a result the memory does not decrease when they’re destroyed.

Question: What is the best way to remove the Gui from the client and reduce the memory usage when doing so? Is there an alternative to Destroy()? Or a method that cleans out the cache?

Images:
When the player is just standing around, before opening GUis:

After opening some GUIs:

The memory being used before/after opening the GUIs goes from 440MB to 820-850MB. I’d like to prevent that if possible. The test version of the game can be played here. Thanks!

Operatik · October 10, 2019, 8:53pm

The possible memory leak might come from scripts. Check if you have bulky loops or event “connections” adding up. I have no idea how your scripts work.

ScribeOfTheEndless · October 10, 2019, 9:24pm

I don’t. I have a single while true loop going. The giant memory increase only happens once the GUIs are opened, and don’t go away when they are Destroyed.

Operatik · October 10, 2019, 9:28pm

Mind sharing the code of how it is opened?

Do a different test this time:

Enable the GUI by default.
Run a game and see if the memory was increased or not.

If the memory does not do a giant leap, it’s the scripts.
Otherwise, the problem is on the UI.

Luaction · October 10, 2019, 10:11pm

If I had to guess you are using some large images for your GUIs which is the reason for your high memory usage in which case Roblox will still keep the preloaded image files in memory even if they aren’t needed anymore as long as the system has enough memory available so the client doesn’t have to keep redownloading the same images every time it opens the GUI.

If you want to actually reduce memory usage for your GUIs you should replace those with primitive GUI elementsas much as possible that don’t require the client to download additional assets from the server as these are much more memory efficient.

ScribeOfTheEndless · October 10, 2019, 10:11pm

Sure!

This is the Menu GUI thats always visible for the player. I’ve abbreviated it to just reflect the ShopGUI button, but the rest of them would be similar:

MenuOpenButton.MouseButton1Click:Connect(function()
    if Screen.Open.Value == false then
    	Screen.Open.Value = true
    	MenuOpenButton.TextLabel.Text = 'CLOSE MENUS'
    	MenuFrame:TweenPosition(UDim2.new(0,0,0,0),"Out","Quint",1.5,true)

--Inside the MenuFrame would be the Shop Button
    ShopBtn.MouseButton1Click:Connect(function()
    	ShopBtn.TextLabel.Text = 'CLOSE SHOP'
    	if ShopOpen.Value == false then
    	local gui = SHOP:Clone()
    	gui.Parent = Plr.PlayerGui 
    	gui.Frame:TweenPosition(UDim2.new(0.3,0,0.2,0),"Out","Quint",1.5,true)
    		ShopOpen.Value = true	
    	else
    		ShopOpen.Value = false
    		local GuiToKill = Plr.PlayerGui:FindFirstChild('SHOPGui')
    		ShopBtn.TextLabel.Text = 'SHOP'
    		GuiToKill:Destroy()
    	end
    end)

    else
-- If Players click the 'Close Menu' button instead of the individual GUIs' close buttons, this line below Destroy()'s the GUIs
    		if ShopBtn.Open.Value == true then ShopBtn.Open.Value = false ShopBtn.TextLabel.Text = 'SHOP' Plr.PlayerGui:FindFirstChild('SHOPGui'):Destroy() ShopOpen.Value = false end

    MenuScript:Destroy()
    MenuScript = ScriptBackup:Clone()
    MenuScript.Parent = Screen
    		
    		
    		Screen.Open.Value = false
    		MenuOpenButton.TextLabel.Text = 'MENUS'
    		MenuFrame:TweenPosition(UDim2.new(-1, 0,0, 0),"Out","Quint",1.5,true)
    	end
    end)

And then inside the GUi thats pulled up, the Shop Gui in this case, is the close button that would ShopGui:Destroy()

As noted, the GUIs are being destroyed, but the memory usage is not decreasing after they’re destroyed that was added once they were opened.

ScribeOfTheEndless · October 10, 2019, 10:13pm

Interesting! Is there a way to force Roblox not to store the images when not needed?

Operatik · October 10, 2019, 10:13pm

There’s one sketchy thing here. That is the connection and it keeps adding up. It’s still connected and will cause memory leak.

ScribeOfTheEndless · October 10, 2019, 10:14pm

What would be the best way to disconnect it?

Lua_Basics · October 10, 2019, 10:26pm

If this is not already:

I’d honestly just use a variable to hold the gui object reference instead of doing this check.

Now,

This wouldn’t cause a memory leak at all. As Flostrus stated, it’s probably due to cached images. Try removing ALL of the image-type objects inside the UI and test it that way.

Luaction · October 10, 2019, 10:26pm

Why would you want that?
It is an optimization by roblox that doesn’t decrease but improves performance.
It will already unload those assets for you if it’s necessary.
In any case if the client has low amounts of memory to begin with it will struggle already when it tries to open that GUI so reduce the amount of assets you use in that file as I described above if you want to actually reduce memory usage.

Destroy() already cleans up all events associated with an Instance and as long as you don’t keep a reference to the Instance or event around it should not cause any memory leaks.

Lua_Basics · October 10, 2019, 10:28pm

From my understanding, doesn’t Destroy already break all references and connections to the object and it’s descendants. You can still stack up connections and events on objects that exist, and that’ll cause the memory to build until the object is destroyed or those are disconnected.

The only real time you need to seriously watch connections and references to objects, would say be for the player objects, as it doesn’t really get destroyed and will stay until everything is disconnected from it.

Operatik · October 10, 2019, 10:30pm

Read twice, ShopBtn:Destroy() was never called and ShopBtn.TextLabel.Text = 'SHOP' leads to a conclusion that the button was never destroyed.

The MouseButton1Click event causes another MouseButton1Click event to add up to the button.

cc @Flostrus

ScribeOfTheEndless · October 10, 2019, 10:32pm

The game does have a lot of image assets, several hundred items that can be purchased by the player, or viewed in their inventory, each a unique image. I’d like for the player to be able to see what they’ve purchased, instead of text, if possible. =/

Roblox being able to hold images in memory cache sounds great, until its the reason why that’s an extra few hundred MB added to the memory usage and causing slowdown on low end laptops and older mobile devices.

Is there no other way around being able to have a large amount of images, other than to remove them?

ScribeOfTheEndless · October 10, 2019, 10:34pm

Thats my bad then for trying to abbreviate the script some in my paste above, the ShopBtn variable is established in the script.

Summary

wait()
local Plr = game.Players.LocalPlayer
local MenuOpenButton = script.Parent.MenuOpenButton
local Screen = script.Parent
local MenuFrame = script.Parent.MenuFrame
local MenuScript = script
local ScriptBackup = MenuScript:Clone()	

MenuOpenButton.MouseButton1Click:Connect(function()

local MENUs = game.ReplicatedStorage:FindFirstChild("MENUs")	
local BUILD = MENUs.BUILDGui
local CODE = MENUs.CodeGui
local GAMES = MENUs.GAMESTELEPORTGui
local INVENTORY = MENUs.INVENTORYGui
local MAGIMON = MENUs.MagiMonStatsGui
local QUEST = MENUs.QuestLog
local SHOP = MENUs.SHOPGui
local TELEPORT = MENUs.SURFACETeleport
local TUTORIAL = MENUs.TUTORIAL
local BuildBtn = MenuFrame.BuildOpenButton
local BuildOpen = MenuFrame.BuildOpenButton.Open
local CodeBtn = MenuFrame.CodeOpenButton
local CodeOpen = MenuFrame.CodeOpenButton.Open
local GamesBtn = MenuFrame.GamesOpenButton
local GamesOpen = MenuFrame.GamesOpenButton.Open
local InvBtn = MenuFrame.InventoryOpenButton
local InvOpen = MenuFrame.InventoryOpenButton.Open
local MagiBtn = MenuFrame.MagimonOpenButton
local MagiOpen = MenuFrame.MagimonOpenButton.Open
local QuestBtn = MenuFrame.QuestOpenButton
local QuestOpen = MenuFrame.QuestOpenButton.Open
local ShopBtn = MenuFrame.ShopOpenButton
local ShopOpen = MenuFrame.ShopOpenButton.Open
local TeleBtn = MenuFrame.TeleportOpenButton
local TeleOpen = MenuFrame.TeleportOpenButton.Open

local TutBtn = MenuFrame.TutorialOpenButton	
local TutOpen = TUTORIAL.Open
	
	
if Screen.Open.Value == false then
	Screen.Open.Value = true
	MenuOpenButton.TextLabel.Text = 'CLOSE MENUS'
	MenuFrame:TweenPosition(UDim2.new(0,0,0,0),"Out","Quint",1.5,true)
			


ShopBtn.MouseButton1Click:Connect(function()
	ShopBtn.TextLabel.Text = 'CLOSE SHOP'
	if ShopOpen.Value == false then
	local gui = SHOP:Clone()
	gui.Parent = Plr.PlayerGui 
	gui.Frame:TweenPosition(UDim2.new(0.3,0,0.2,0),"Out","Quint",1.5,true)
		ShopOpen.Value = true	
	else
		ShopOpen.Value = false
		local GuiToKill = Plr.PlayerGui:FindFirstChild('SHOPGui')
		ShopBtn.TextLabel.Text = 'SHOP'
		GuiToKill:Destroy()
	end
end)

	else

if ShopBtn.Open.Value == true then ShopBtn.Open.Value = false ShopBtn.TextLabel.Text = ‘SHOP’ Plr.PlayerGui:FindFirstChild(‘SHOPGui’):Destroy() ShopOpen.Value = false end

MenuScript:Destroy()
MenuScript = ScriptBackup:Clone()
MenuScript.Parent = Screen
		
		
		Screen.Open.Value = false
		MenuOpenButton.TextLabel.Text = 'MENUS'
		MenuFrame:TweenPosition(UDim2.new(-1, 0,0, 0),"Out","Quint",1.5,true)
	end
end)

Lua_Basics · October 10, 2019, 10:39pm

I did read again, the mainframe which has the connection to the button is ALWAYS in the player’s UI. Now he clones a gui from replicated storage and will destroy this exact gui. When that gui is destroyed the memory never goes down after that particular gui was opened.

So please read again.

Here’s one of your problems though, you check to see if it’s closed but never disconnect the button:

So you’re setting multiple connections to this one button?

Operatik · October 10, 2019, 10:41pm

This was the most unholy script I have ever seen.

Fixed Indents - NOT SOLUTION

MenuOpenButton.MouseButton1Click:Connect(function()
	local MENUs = game.ReplicatedStorage:FindFirstChild("MENUs")	
	local BUILD = MENUs.BUILDGui
	local CODE = MENUs.CodeGui
	local GAMES = MENUs.GAMESTELEPORTGui
	local INVENTORY = MENUs.INVENTORYGui
	local MAGIMON = MENUs.MagiMonStatsGui
	local QUEST = MENUs.QuestLog
	local SHOP = MENUs.SHOPGui
	local TELEPORT = MENUs.SURFACETeleport
	local TUTORIAL = MENUs.TUTORIAL
	local BuildBtn = MenuFrame.BuildOpenButton
	local BuildOpen = MenuFrame.BuildOpenButton.Open
	local CodeBtn = MenuFrame.CodeOpenButton
	local CodeOpen = MenuFrame.CodeOpenButton.Open
	local GamesBtn = MenuFrame.GamesOpenButton
	local GamesOpen = MenuFrame.GamesOpenButton.Open
	local InvBtn = MenuFrame.InventoryOpenButton
	local InvOpen = MenuFrame.InventoryOpenButton.Open
	local MagiBtn = MenuFrame.MagimonOpenButton
	local MagiOpen = MenuFrame.MagimonOpenButton.Open
	local QuestBtn = MenuFrame.QuestOpenButton
	local QuestOpen = MenuFrame.QuestOpenButton.Open
	local ShopBtn = MenuFrame.ShopOpenButton
	local ShopOpen = MenuFrame.ShopOpenButton.Open
	local TeleBtn = MenuFrame.TeleportOpenButton
	local TeleOpen = MenuFrame.TeleportOpenButton.Open

	local TutBtn = MenuFrame.TutorialOpenButton	
	local TutOpen = TUTORIAL.Open

	if Screen.Open.Value == false then
		Screen.Open.Value = true
		MenuOpenButton.TextLabel.Text = 'CLOSE MENUS'
		MenuFrame:TweenPosition(UDim2.new(0,0,0,0),"Out","Quint",1.5,true)

		BuildBtn.MouseButton1Click:Connect(function()
			BuildBtn.TextLabel.Text = 'CLOSE BUILD'
			if BuildOpen.Value == false then
			local gui = BUILD:Clone()
			gui.Parent = Plr.PlayerGui 
			gui.BuildFrame:TweenPosition(UDim2.new(0.294,0,0,0),"Out","Quint",1.5,true)
				BuildOpen.Value = true	
			else
				BuildOpen.Value = false
				local GuiToKill = Plr.PlayerGui:FindFirstChild('BUILDGui')
				BuildBtn.TextLabel.Text = 'BUILD'
				GuiToKill:Destroy()
			end
		end)

		CodeBtn.MouseButton1Click:Connect(function()
			CodeBtn.TextLabel.Text = 'CLOSE CODES'
			if CodeOpen.Value == false then
			local gui = CODE:Clone()
			gui.Parent = Plr.PlayerGui 
			gui.CodeFrame:TweenPosition(UDim2.new(0.5, -200, 0.5, -150),"Out","Quint",1.5,true)
				CodeOpen.Value = true	
			else
				CodeOpen.Value = false
				local GuiToKill = Plr.PlayerGui:FindFirstChild('CodeGui')
				CodeBtn.TextLabel.Text = 'CODES'
				GuiToKill:Destroy()
			end
		end)

		GamesBtn.MouseButton1Click:Connect(function()
			GamesBtn.TextLabel.Text = 'CLOSE GAMES'
			if GamesOpen.Value == false then
			local gui = GAMES:Clone()
			gui.Parent = Plr.PlayerGui 
			gui.GamesTeleportFrame:TweenPosition(UDim2.new(0.072, 0,0, 0),"Out","Quint",1.5,true)
				GamesOpen.Value = true	
			else
				GamesOpen.Value = false
				local GuiToKill = Plr.PlayerGui:FindFirstChild('GAMESTELEPORTGui')
				GamesBtn.TextLabel.Text = 'GAMES'
				GuiToKill:Destroy()
			end
		end)

		InvBtn.MouseButton1Click:Connect(function()
			InvBtn.TextLabel.Text = 'CLOSE INVENTORY'
			if InvOpen.Value == false then
			local gui = INVENTORY:Clone()
			gui.Parent = Plr.PlayerGui 
			gui.InventoryFrame:TweenPosition(UDim2.new(0.3,0,0.2,0),"Out","Quint",1.5,true)
				InvOpen.Value = true	
			else
				InvOpen.Value = false
				local GuiToKill = Plr.PlayerGui:FindFirstChild('INVENTORYGui')
				InvBtn.TextLabel.Text = 'INVENTORY'
				GuiToKill:Destroy()
			end
		end)

		MagiBtn.MouseButton1Click:Connect(function()
			MagiBtn.TextLabel.Text = 'CLOSE MagiMon STATS'
			if MagiOpen.Value == false then
			local gui = MAGIMON:Clone()
			gui.Parent = Plr.PlayerGui 
			gui.Frame:TweenPosition(UDim2.new(0.254,0,0,0),"Out","Quint",1.5,true)
				MagiOpen.Value = true	
			else
				MagiOpen.Value = false
				local GuiToKill = Plr.PlayerGui:FindFirstChild('MagiMonStatsGui')
				MagiBtn.TextLabel.Text = 'MagiMon STATS'
				GuiToKill:Destroy()
			end
		end)

		QuestBtn.MouseButton1Click:Connect(function()
			QuestBtn.TextLabel.Text = 'CLOSE QUEST LOG'
			if QuestOpen.Value == false then
			local gui = QUEST:Clone()
			gui.Parent = Plr.PlayerGui 
			gui.SettingsFrame:TweenPosition(UDim2.new(0.136, 0,0.065,0),"Out","Quint",1.5,true)
				QuestOpen.Value = true	
			else
				QuestOpen.Value = false
				local GuiToKill = Plr.PlayerGui:FindFirstChild('QuestLog')
				QuestBtn.TextLabel.Text = 'QUEST LOG'
				GuiToKill:Destroy()
			end
		end)

		ShopBtn.MouseButton1Click:Connect(function()
			ShopBtn.TextLabel.Text = 'CLOSE SHOP'
			if ShopOpen.Value == false then
			local gui = SHOP:Clone()
			gui.Parent = Plr.PlayerGui 
			gui.Frame:TweenPosition(UDim2.new(0.3,0,0.2,0),"Out","Quint",1.5,true)
				ShopOpen.Value = true	
			else
				ShopOpen.Value = false
				local GuiToKill = Plr.PlayerGui:FindFirstChild('SHOPGui')
				ShopBtn.TextLabel.Text = 'SHOP'
				GuiToKill:Destroy()
			end
		end)

		TeleBtn.MouseButton1Click:Connect(function()
			TeleBtn.TextLabel.Text = 'CLOSE TELEPORT'
			if TeleOpen.Value == false then
			local gui = TELEPORT:Clone()
			gui.Parent = Plr.PlayerGui 
			gui.SettingsFrame:TweenPosition(UDim2.new(0.23, 0,0.065, 0),"Out","Quint",1.5,true)
				TeleOpen.Value = true	
			else
				TeleOpen.Value = false
				local GuiToKill = Plr.PlayerGui:FindFirstChild('SURFACETeleport')
				TeleBtn.TextLabel.Text = 'TELEPORT'
				GuiToKill:Destroy()
			end
		end)

		TutBtn.MouseButton1Click:Connect(function()
			TutBtn.TextLabel.Text = 'CLOSE TUTORIAL'
			if TutOpen.Value == false then
			local gui = TUTORIAL:Clone()
			gui.Parent = Plr.PlayerGui 
			gui.HowToWinFrame:TweenPosition(UDim2.new(0.004, 0,0.009, 0),"Out","Quint",1.5,true)
				TutOpen.Value = true	
			else
				TutOpen.Value = false
				local GuiToKill = Plr.PlayerGui:FindFirstChild('TUTORIAL')
				TutBtn.TextLabel.Text = 'TUTORIAL'
				GuiToKill:Destroy()
			end
		end)


		-- BELOW IS THE CLOSE MENUS BUTTON ACTIONS

	else
		if BuildBtn.Open.Value == true then BuildBtn.Open.Value = false BuildBtn.TextLabel.Text = 'BUILD' Plr.PlayerGui:FindFirstChild('BUILDGui'):Destroy() BuildOpen.Value = false end
		if CodeBtn.Open.Value == true then CodeBtn.Open.Value = false CodeBtn.TextLabel.Text = 'CODES' Plr.PlayerGui:FindFirstChild('CodeGui'):Destroy() CodeOpen.Value = false end
		if GamesBtn.Open.Value == true then GamesBtn.Open.Value = false GamesBtn.TextLabel.Text = 'GAMES' Plr.PlayerGui:FindFirstChild('GAMESTELEPORTGui'):Destroy() GamesOpen.Value = false end
		if InvBtn.Open.Value == true then InvBtn.Open.Value = false InvBtn.TextLabel.Text = 'INVENTORY' Plr.PlayerGui:FindFirstChild('INVENTORYGui'):Destroy() InvOpen.Value = false end
		if MagiBtn.Open.Value == true then MagiBtn.Open.Value = false MagiBtn.TextLabel.Text = 'MagiMon STATS' Plr.PlayerGui:FindFirstChild('MagiMonStatsGui'):Destroy() MagiOpen.Value = false end
		if QuestBtn.Open.Value == true then QuestBtn.Open.Value = false QuestBtn.TextLabel.Text = 'QUEST LOG' Plr.PlayerGui:FindFirstChild('QuestLog'):Destroy() QuestOpen.Value = false end
		if ShopBtn.Open.Value == true then ShopBtn.Open.Value = false ShopBtn.TextLabel.Text = 'SHOP' Plr.PlayerGui:FindFirstChild('SHOPGui'):Destroy() ShopOpen.Value = false end
		if TeleBtn.Open.Value == true then TeleBtn.Open.Value = false TeleBtn.TextLabel.Text = 'TELEPORT' Plr.PlayerGui:FindFirstChild('SURFACETeleport'):Destroy() TeleOpen.Value = false end
		if TutBtn.Open.Value == true then TutBtn.Open.Value = false TutBtn.TextLabel.Text = 'TUTORIAL' Plr.PlayerGui:FindFirstChild('TUTORIAL'):Destroy() TutOpen.Value = false end

		MenuScript:Destroy()
		MenuScript = ScriptBackup:Clone()
		MenuScript.Parent = Screen
		
		Screen.Open.Value = false
		MenuOpenButton.TextLabel.Text = 'MENUS'
		MenuFrame:TweenPosition(UDim2.new(-1, 0,0, 0),"Out","Quint",1.5,true)
	end
end)

You could have declared the variables in advance first. Then do a function for each button. Do not destroy any UI, set their visible to false(they won’t render).

The real question is why would you clone and destroy when you can actually show and hide.

ScribeOfTheEndless · October 10, 2019, 10:44pm

I don’t mind the lack of indents, I follow it fine, but thank you for doing so here.

I tried hiding/showing, didn’t decrease the amount of memory usage, at all.

Before the GUIs are cloned, it runs at 440MB. After the GUis are cloned and opened, it runs at 820-850MB.

If I don’t clone them, and just nestle them all under the one GUI with show/hides, then the Memory used sits at 850MB. I’d like to use less memory, hence Cloning them when I need them, and Destroying them when they’re not needed.

Lua_Basics · October 10, 2019, 10:45pm

This is your exact problem OP, at least one of them. When the button is clicked you set the connections to the event instead of setting them before-hand and using variable checks. Also, as Operatik said you could easily just hide the UI instead of cloning and destroying everything.

Also, everything after this won’t be ran OP.

Luaction · October 10, 2019, 10:46pm

Destroy() only disconnects all events and locks its Parent but it doesn’t destroy the references
For example:

local p = Instance.new("Part")
p:Destroy()
print(p) --will print "Part"

You can also create a memory leak with Destroy()

local p = Instance.new("Part")
p:Destroy()
print(p)
while wait() do --This will cause a memory leak as the reference to p will never get cleaned up
--do stuff
end

Aside from setting p to nil you can also do this to fix the memory leak:

do --this will prevent the memory leak from happening
	local p = Instance.new("Part")
	p:Destroy()
	print(p.Name, p.CFrame)
end
print(p) --this will now print nil as the variable "p" no longer exists

while wait() do 
--do stuff
end