Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

concat number with string to convert it doesn't typecheck by default, but it's faster than tostring() #777

Closed
matthargett opened this issue Dec 7, 2022 · 2 comments · Fixed by #992
Assignees
Labels
enhancement New feature or request

Comments

@matthargett
Copy link

benchmarking

local x = 5
x ..= ""

versus

local x = 5
x = tostring(x)

the ..= version is ~100us faster than tostring(), even though ..= doesn't typecheck and isn't as idiomatic.

@matthargett matthargett added the enhancement New feature or request label Dec 7, 2022
@boatbomber
Copy link

boatbomber commented Dec 7, 2022

Backing up that "faster" claim: (the 100us metric in OP was with N=10k, I've bumped it to 20k in the screenshot to show it more easily at a glance)

image

Using the following to test:

local N = 20_000
return {
	ParameterGenerator = function()
		local nums = table.create(N)
		for i=1, N do
			nums[i] = math.random(1, 10000000)
		end
		return nums
	end,

	Functions = {
		["concat"] = function(Profiler, nums)
			for _, num in nums do
				num ..= ""
			end
		end,

		["tostring"] = function(Profiler, nums)
			for _, num in nums do
				num = tostring(num)
			end
		end,

		["format"] = function(Profiler, nums)
			for _, num in nums do
				num = string.format("%f", num)
			end
		end,
	},
}

@zeux
Copy link
Collaborator

zeux commented Feb 13, 2023

I think that's just a performance bug. We don't have a fastcall for tostring/tonumber - we absolutely can add that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

Successfully merging a pull request may close this issue.

3 participants