Concerning core script performance

There are two CoreScripts in particular that rely on bad practices to function as intended. This has drastic side effects and consequences in our games.

Both of the following scripts rely on polling an instance for the expected instance. This behavior needs to be adjusted to rely on events to prevent the current high memory use and performance costs from happening.

VehicleHud.lua
This script relies on a while loop that searches the children of the character to find the humanoid. Instead this should be calling character:FindFirstChildWhichIsA("Humanoid") and if it doesn’t exist, use character.DescendantAdded() to wait for it. This polling behavior causes frame drops in games that have characters with no humanoid and hundreds of parts.

local function getHumanoid()
	local character = LocalPlayer and LocalPlayer.Character
	if character then
		for _,child in pairs(character:GetChildren()) do
			if child:IsA('Humanoid') then
				return child
			end
		end
	end
end

local function connectSeated()
	local humanoid = getHumanoid()
	while not humanoid do
		wait()
		humanoid = getHumanoid()
	end
	humanoid.Seated:connect(onSeated)
end

ServerLeaderstats.Lua
This script loops through all children of player, and all the children of all of the top level instances, and yields infinitely until object gets removed. There are better methods available to wait for an instance to be removed, but it doesn’t even seem like this code needs to be listening to all instances inside of the player. These yields build up over time as there are more instances under player, leading to high memory usage.

local function playerChildAdded(instance)
	if not isValidContainer(instance) then
		return
	end
	local nameChanged = instance:GetPropertyChangedSignal("Name"):Connect(function ()
		playerChildNameChanged(instance)
	end)
	fastSpawn(playerChildNameChanged, instance)
	waitUntilRemoved(instance)
	nameChanged:Disconnect()
	if instance.Name == LEADERSTATS_NAME then
		leaderstatsRemoved(instance)
	end
end

local function playerAdded(player)
	local childAdded = player.ChildAdded:Connect(playerChildAdded)
	for _, instance in ipairs(player:GetChildren()) do
		fastSpawn(playerChildAdded, instance)
	end
	waitUntilRemoved(player)
	childAdded:Disconnect()
end
14 Likes

An interesting idea for VehicleHud is to yield the main thread and then resume it as soon as a humanoid is found in the character model (with childadded)

local function GetHumanoid()
  local character = LocalPlayer and LocalPlayer.Character
  if character then
    local human = character:FindFirstChildWhichIsA("Humanoid")
    if human then return human end

    local co = coroutine.running()
    local e; e = character.ChildAdded:Connect(function(newChild)
      if newChild:IsA("Humanoid") then
        e:Disconnect()
        task.spawn(co, newChild)
      end
    end)

    return coroutine.yield()
  end
end

This sort of code is a little unorthodox (I’m not a fan of using bindables) but its a better solution than what Roblox are doing

In Roblox’s defence, these are two of the oldest scripts in Roblox (it still uses connect and wait)

Problems will also come up from infinitely yielding for a humanoid to exist. The solution needs to be restructuring of the code to be fully dependent on events to run code on objects that don’t exist yet.

I am not sure how leaderstats could be improved significantly. What they are doing seems pretty reasonable to me. The script needs to detect when an instance called ‘leaderstats’ is parented to any of the players in the experience so it listens to the events that it needs to. They could have taken a different approach but it is not polling in the same way the humanoid example is.

There is a subtle bug with it however. If an instance is parented to the player object and then moved elsewhere without being removed then the thread will continue to yield until then. The quick-fix would be to change the implementation of waitForRemoved in playerChildAdded to return when the object is no longer parented to the player.

Thanks for the report. I filed a ticket to our internal database.

1 Like

We are working on fixes for both of these.
Thanks for reporting!

3 Likes

I have shipped fixes for both of these issues!

3 Likes

This topic was automatically closed 14 days after the last reply. New replies are no longer allowed.