TimespanParse function

The code creates a countdown using UNIX (or os.time())
https://i.gyazo.com/9bb8a82115d3e65450ffe4a14e428788.mp4

Is there anyway this system can be improved? Would it be worth improving?

I think a better improvement could happen where the string is constructed.

function TimespanParse(TotalSeconds)
	local Hours    = math.floor(math.fmod(TotalSeconds, 86400) / 3600)
	local Minutes  = math.floor(math.fmod(TotalSeconds, 3600) / 60)
	local Seconds  = math.floor(math.fmod(TotalSeconds, 60))
	local String = ""
	if (Hours > 0 and Hours < 60) then
		String = String .. Hours .. 'h '
	end
	if (Minutes > 0 and Minutes < 60) then
		String = String .. Minutes .. 'm '
	end
	if (Seconds > 0 and Seconds < 60) then
		String = String .. Seconds .. 's'
	end
	return String
end

-- Usage

local time = os.time() + 600

while wait(0.5) do
	script.Parent.Parent.Closing.Text = TimespanParse(time - os.time())
end

MAJOR UPDATE (2021-03-14)


Introduction

In this updated post, I complete the previous version of my answer with a collection of all the information we’ve gathered throughout this topic, and briefly mention all the improvement aspects we’ve discussed.

The timer function introduced by original poster doesn’t need any serious improvements. It is efficient, and almost any further changes can be interpreted as micro-optimization.

Minor improvements

This sub-section includes only the easy, necessary and/or recommended changes, targeting process time, readability, touches accuracy, and code consistency.

1. time and string naming

Variables shouldn’t be named “string” or “time”. Even though such variables may be used to store data, they are generally reserved for built in functions and lua globals. time is abbrevation for os.time(), and string stores string library methods (similar to table library).

Example:

string.find()   ;   table.insert()

2. Defining variables

It’s a good practice to define variables outside rapidly running functions. Performance difference is likely negligible, but it’s a plus from readability aspect. Only apply changes to define variables through loops.

3. Functions and loops

@DarthFS mentioned making the code more compact and removing abstraction by not calling functions rapidly. That is a good idea, especially in this case, but the final choice depends on complexity of the process. Often enough carrying out the whole process in one function reduces readability in exchange for minor improvement. Code may appear much longer. It’s best to avoid calling function rapidly, but it’s not at all critical if we don’t pass large parameters along.

Read more about call stack here: https://www.learncpp.com/cpp-tutorial/the-stack-and-the-heap/ (link by DarthFS).

Filling call stack with unnecessary large data increases the chances of so called stack overflow happening.

4. os.time() vs. os.clock()

@PysephDEV yes, os.time() is generally a good choice for timers. However, os.clock() is very useful not only for benchmarking, but for general precise use as well. For instance when we are creating a countdown timer displaying minutes till an event (e.g. countdown starting with 5 minutes).

The Roblox Dev Hub currently describes os.clock() as useful mainly when benchmarking, but its role has become broader.

Since tick() has been recently deprecated, we don’t have any other methods that could return us precise delta time. os.time() displays seconds only, so os.clock() is now used for any operation requiring relatively rigorous precision.

I don’t see any serious reasons why not to use os.clock() in processes. Perhaps it’s not necessary in this case, but the important evaluation to do is determine what we are really trying to achieve.

We aren’t calling os.time() to format times, but instead calculating difference (time delta) between the starting point and current time. Using os.clock() and os.time() is done in the same manner, except one result is more precise than the other.

Some quick benchmarking:

os.clock() --> around 1.5 * 10^7s
os.time() --> around 4 * 10^7s
--> No visible performance loss; difference is completely negligible.

5. Waiting time

wait() function with time argument in brackets is very useful, but is not completely accurate. It’s part of 30Hz pipeline, yields the code, and runs every second rendered frame at best. It isn’t always reliable or consistent, hence occasional delays, such as:

wait(0.5) --> 0.501, 0.507, 0.517, 0.56 ; up to seconds in some cases

Wait time really depends on other scripts as well. These delays don’t affect your timer much, because you are not displaying miliseconds. With waiting time of 0.5s, you may experiency delays in display (e.g. time = 5s, printed at 4.9s or 4.8s), which is not to be confused with displayed time accuracy. Why?

Because you are calculating the difference between starting time and current time. The time we are calculating the difference at has no impact on accuracy.

Conclusion of this subsection: rather reduce waiting time to around 0.3s, meaning your function will run around 3 times. Display time in seconds will look smooth in most cases.

Recommended script

local START_TIME = os.time() + 600

local label = script.Parent.Parent.Closing -- shorten paths
local hours, minutes, seconds, total, text

local function countDown()
	repeat
		total = START_TIME - os.time()
		text = ""
		
		hours    = math.floor(math.fmod(total, 86400) / 3600)
		minutes  = math.floor(math.fmod(total, 3600) / 60)
		seconds  = math.floor(math.fmod(total, 60))
		
		-- Note: string ..= "happy" is the same as string = string .."happy"
		if (hours > 0 and hours < 60) then text ..= hours ..'h ' end
		if (minutes > 0 and minutes < 60) then text ..= minutes ..'m ' end
		if (seconds > 0 and seconds < 60) then text ..= seconds ..'s' end
		
		-- Task
		label.Text = text
		wait(.3)
	until total <= 0
end

-- Usage
countDown()

Precision - additional information

Throughout the topic, a discussion about my mentioning of RunService.Heartbeat has been discussed. I mentioned it purely as an extra tip if you ever decided to display miliseconds. There have been some misunderstandings regarding the idea.

If precision matters, RunService.Heartbeat is the right choice. This applies for displaying in miliseconds.

Expand this fold to read more!

Now, I accidentally told some wrong information here: Heartbeat doesn’t run at the rate of 30Hz only. That used to be true in the past, but doesn’t apply today. It is, however, much more consistent than wait(). Heartbeat, as @DarthFS said, depends on game frame rate (FPS), although if no serious stuttering is present and game performs well, Heartbeat:Wait() is pretty consistent. There is a big framerate drop when client joins or server starts, but consistency is established quickly.

Note: Heartbeat will be replaced by its successor PostSimulation in the near future.

What is Heartbeat?

Heartbeat is an item, an event that fires every frame after game physics simulation. Delta time (step) is equal to the time elapsed since the previous frame.

Using Heartbeat:Wait() in your current situation would be a big waste of resources. I never said you should use Heartbeat in your function, only if you added miliseconds. Why Heartbeat for miliseconds?

--[[
	Heartbeat:Wait() time according to framerate:
		50 fps --> =~ 1/50s
	
	Display:
]]
00 : 00: 00: 00 --> hours, minutes, seconds, miliseconds

Heartbeat is necessary in this case. Just extending the example here.




Micro optimization

We’ve now covered all the most important “angles”. There isn’t much to improve now. Optimization on micro-level is not always a good idea, for the reasons explained by both PysephDEV and DarthFS.

Premature optimization in a nutshell is a technique often slightly improving the code, but sacrifices readability. Sometimes called literate programming is a principle of writing neat code whoose primary goal is to be easily understandable by at least an intermediate human reader.

Is premature optimization welcome? Usually not, at least not in Roblox environment. Difficult code is often difficult to manage. Rarely is optimal performance so crucial that we decide to sacrifice code readability.

However, I can’t completely agree with the statement “premature optimization is the root of evil” in this forum section. The purpose of code review is to review the code and discuss different approaches. Almost anything is welcome. String concatenation is not critical here at all, but is a good discussion opener. So no, I don’t see any problem in mentioning it. In fact, it’s an advantage, as @blue1010123 and anyone else reading might learn something new.

String concatenation

In lua, we can concatenate strings using the dot (…) operator. Everything is perfectly fine, as long as we don’t overdo it, as string concatenation to some extent negatively affects performance. Even though I already used this quote below, it seems reasonable to repeat it here.

… there is a caveat to be aware of. Since strings in Lua are immutable, each concatenation creates a new string object and copies the data from the source strings to it. That makes successive concatenations to a single string have very poor performance.
( - RBerteig, Stack Overflow)

The rest of this post is intended for comparision of various ways to concatenate strings.

I won’t be performing any tests, because they are already described in other posts, but briefly explain each method.

How does string concatenation work in lua?

local ourString = "" --> string created
ourString = ourString .. "What " --> new string created
ourString = ourString .. "a " --> new string created
... -- same process until
ourString = ourString .. "world!" --> new string defined again

-- Altogether: 5 individual variations in memory for one sentence.

Again, not at all bad when we are gradually combining a couple of strings, but relatively bad when we have a lot of them.

Each time a string is created, it occupies a certain place in memory. It’s a very small place, so not much compared to the overall memory size, albeit efficient code is very important, because memory is still limited.

  1. One-line concatenation
local ourString = "What ".."a ".."wonderful ".."world!"

Such concatenation was the most performant in tests. Only one string is created very quickly. The only disadvantage is manual concatenation. There may be too many strings to combine them in one line. Spliting it into multipe smaller strings is also alright. But what if we have 20 or even hundreeds of strings?

  1. Concatenation using a loop and dot operator

To test how performant gradual concatenation is, I concatenated 10, 20, 1000, and 2.5 * 10^5 randomly generated letters (shortest possible strings).

As expected, string concatenation performed badly, because we created 250 000 strings to format the final one. Memory usage increased by more than 130 mb.

  1. Concatenation using a loop and table.concat

As an alternative, the idea was to concatenate strings using tables. Previously, the process involved relatively unnecessary step of creating a new table, inserting characters into it and concatenating elements of the newly created table. Luckily, we can skip that step, and simply store concatenated result of orignal array. The precondition is to have strings stored in form of a table.

Results and conclusions

One-line concatenation seems ideal for combining lower number of strings.
Concat. in multiple lines is acceptable as well, but creates a couple of more strings along the way.
Both methods are straightforward and simple yet efficient. It is about 2 times faster than table method.

Dot operator performs poorly when a lot of strings need to be processed. One-line concatenation is just as efficent, despite being impractical in such cases.

Table method performs best with higher numbers of strings, is significantly faster and performance friendlier. When used for processing of lower number of strings, for example 5, it is a better option compared to gradual string concat., but slower than one-line concatenation.

The table method is far from “hideous”, even though it seems so at first sight. Copying to another table was unnecessary and worse option than plain table.concat, but still quite performant.

Code to compare table.concat and gradual concat methods
local NUMBER_OF_STRINGS = 5

local array = {}
local char, t, s

for i = 1, NUMBER_OF_STRINGS do
	char = math.random(97, 122)
	table.insert(array, string.char(char))
end

local tableTime, dotTime

-- STRING CONCATENATION --------------
dotTime = os.clock()
s = ""
for _, v in ipairs(array) do
	s = s .. v
end
dotTime = os.clock() - dotTime
--------------------------------------

-- TABLE CONCATENATION ---------------
tableTime = os.clock()
t = table.concat(array, "")
tableTime = os.clock() - tableTime
--------------------------------------
--------------------------------------	
print(tableTime, dotTime)
print(( tableTime < dotTime and 'Table concat. method was faster.' ) or 'String concat. method was faster.')
Code to compare all three methods on a short string
local strTime_sing, strTime_mult, tableTime, t, s, txt

local array = {"This ", "is ", "a ", "long ", "sample ", "string", "!", " =)"}

for i = 1, 500 do
	-- TABLE CONCATENATION -----------------------------------------------------
	tableTime = os.clock()
	t = table.concat(array, "")
	tableTime = os.clock() - tableTime
	----------------------------------------------------------------------------
	
	-- SINGLE LINE CONCATENATION -----------------------------------------------
	strTime_sing = os.clock()
	txt = "This ".."is ".."a ".."long ".."sample ".."string".."!".." =)"
	strTime_sing = os.clock() - strTime_sing
	----------------------------------------------------------------------------
	
	-- GRADUAL CONCATENATION ---------------------------------------------------
	strTime_mult = os.clock()
	s = ""
	for _, v in ipairs({"This ", "is ", "a ", "long ", "sample ", "string", "!", " =)"}) do
		s = s .. v
	end
	strTime_mult = os.clock() - strTime_mult
	----------------------------------------------------------------------------
end

print("Single line:".. strTime_sing ..", multiple line: ".. strTime_mult ..", table: ".. tableTime)

local min = math.min(strTime_sing, strTime_mult, tableTime)
print(
	min == strTime_sing and "single string concat" or
		min == strTime_mult and "multiple string concat" or
		"table concat"
)

Alternative script with included micro optimization

local START_TIME = os.time() + 600

local label = script.Parent.Parent.Closing -- shorten paths
local hours, minutes, seconds, total, x

local function countDown()
	repeat
		total = START_TIME - os.time()
		hours    = math.floor(math.fmod(total, 86400) / 3600)
		minutes  = math.floor(math.fmod(total, 3600) / 60)
		seconds  = math.floor(math.fmod(total, 60))
		
		if (hours > 0 and hours < 60) then x = 3
		elseif (minutes > 0 and minutes < 60) then x = 2
		elseif (seconds > 0 and seconds < 60) then x = 1
		end
		
		-- Read further to see what's happening in the following statement.
		label.Text = ((x > 2) and hours .. 'h ' or '') .. ((x > 1) and minutes .. 'm ' or '') .. seconds .. 's'
		wait(.3)
	until total <= 0
end

-- Usage
countDown()

This post got so long that I’ll have to update it later.

In the final part, let’s explain what what short-circuit evaluation is (used above).

Short-circuit evaluation (also known as minimal evaluation and McCarthy evaluation) is a special semantics typical for some programmig languages. Structure in lua:

local a = true
local variable = a and "a is true" or "a isn't true"
print(variable)

Only if the first condition is met, the second argument is executed. Otherwise, script continues with alternative argument (after or).

The above code is equivalent to

local a = true

if (a == true) then
	variable = "a is true"
else
	variable = "a isn't true"
end
print(variable)

Short-circuit evaluation is very useful, because it’s short and simple. Above case shows a little more complicated use formed this way:

local x = condition1 and value1 or condition2 and value2 or value3

We can do multiple evaluations. How else can we use it?

local object = workspace:FindFirstChild("Part")
local name = object and object.Name or "default"
-- respectively
local name = (object == true) and object.Name or "default"

Hopefully you find this post useful @blue1010123, it took quite some time to write. I bet learning something new is better than getting some small corrections on your code back accompanied with a statement saying that there is nothing much to improve, and/or the function is written optimally. If any of the information seems confusing or you have any questions, feel free to contact me via private message. You can of course ignore some corrections. This post is meant to be informative after all.

3 Likes

Thank you very much for the detailed reply. This information is very useful! I love learning about micro performance as it’s very interesting to me.

1 Like

I am a bit concerned about your use of short-circuit evaluation impacting readability. Normally I would have a maximum or 3 and / or booleans before I consider it over-excessive. I also feel like your tip on not using concatenation is also extra.

The memory allocation is really nothing to worry about, once the function that’s called terminates, all memory that was allocated in that function gets freed. Concatenation is very common practice in many programming languages (low or high level). Concatenation affects almost 0 performance. A few extra bytes wouldn’t hurt.

This is also incorrect, heartbeat is proportionate to the client / game FPS meaning it’s not always a certain number.

In addition, your version is less performant than the original. It may be more accurate, but you are still calling that function around 30x more quickly than previously (well “30x” depends on your FPS) which is a much more serious optimization when it comes to flooding the call stack with extraneous function calls. (Yielding less time means more function calls).

What I would recommend is to not have a function at all since it’s an unnecessary level of abstraction. Just put all that code in the loop because that’s the only place where you are running that chunk of code.

while true do
   wait(0.3)
   -- Paste the function code in
end

In terms of cognitive complexity, I wouldn’t recommend that approach. It’s not always about micro-optimizations but also readability. If you were working on a team, it would be very hard for them to debug your code.

Which is why micro-optimizations are “the root of all evil”:

2 Likes

Hi @DarthFS!

Concatenating string

I understand you concern. Premature optimization is indeed not a good decision. However, in my defence, the statement I used

may look a little more compact and slightly less readable than what we had before, but is no rocket science, and anyone can understand what the function does in no more than 30 seconds. The return is simply concatenated result, so the reader knows what the return is going to be.

As far as optimization goes, string concatenation in fact does negatively affect performance to some extent. A reputable member of Stack Overflow describes it pretty well:

… there is a caveat to be aware of. Since strings in Lua are immutable, each concatenation creates a new string object and copies the data from the source strings to it. That makes successive concatenations to a single string have very poor performance.
( - RBerteig, Stack Overflow)

Idea: concatenating using tables

That is why working carefully with concatenation is important; that is why I used concatenation in one run. Maybe it has no positive effect, but logically seems so. If micro-optimization played a key role, we should have perhaps considered the benefits of using the following function (written by above user as well).

local function concatStrings(array)
	local t = {}
	for _, v in ipairs(array) do
		t[#t+1] = tostring(v)
	end
	return table.concat(t,"")
end

print(concatStrings( {"What ", "a ", "wonderful ", "world!"} ))

We are still dealing with concatenation, so there likely isn’t much performance difference.

It is extra but isn’t at the same time. Memory allocation is not to be neglected when programming. The case we are talking about doesn’t really involve a demanding process. Nevertheless, GC (garbage collector) does clear the memory, but it does it in periodically. Memory clearing is not instant. Let’s move to RunService.Heartbeat:Wait() issue.

Heartbeat when precision is necessary

First of all, thank you for correcting me on Heartbeat run rate information. Heartbeat depends on game frame rate. What is said used to be true some time ago. However, Heartbeat is much more consistent than wait(). When game starts, there is a framerate drop. Most of the time after that, consistency is established again.

It’s not “my version”. I mentioned Heartbeat, because it’s the best option when dealing with miliseconds and precision becomes important. I didn’t say that anyone should use it in this case.

If precision matters, rather use RunService.Heartbeat:Wait() .

Why best option in that case? Because the number decrease looks much smoother. Ideally, when displaying miliseconds. Event Heartbeat is theoretically not accurate enough.

About function calls? I don’t see any problems with calling a function rapidly. These are not even very frequent calls. Event when using :BindToRenderStepped, which runs before the frame is rendered, calling lighter function is not an issue. But that’s right, you can avoid abstraction, but often at the cost of readability.

For countdowns, don’t even use while-loops. Repeat loops seem like a better option. Now, don’t do what I did in the following function, because I’m simply using wait(n) to change a condition. Don’t make functions depend on external conditions like this either. Only take a look at the calls as an example.

Repeat-loop
local countdown = true

local function sampleLoop()
	repeat
		wait(.1); print("OK")
	until not countdown
end
coroutine.wrap(sampleLoop)()
wait(5)
countdown = false

RunService.Heartbeat
local sampleLoop = game:GetService("RunService").Heartbeat:Connect(function()
	print("OK")
end)
wait(5)
sampleLoop:Disconnect()

@DarthFS thank you for contributing to this discussion :slight_smile:.

@blue1010123 you may want to read this as well.

PS: doing multiple string concatenations is not bad in this case, but is generally to be avoided. Run rate should be at least atound 2.5 per second, so reduce wait time to 0.3 s.

Readability is often a subjective matter but there is usually a baseline. Your recommendation on short-circuit evaluation is based on negligible speed over readability. When having a professional scripting job, it’s important to be productive yet optimize to a certain extent. When you don’t focus on optimizing your code to a factor of 0.00001, you can spend your time finishing another part of your game saving yourself time, reducing your chances of getting fired.

No offense, but being nice, this function is hideous. Basically you are allocating a table in memory by having an array argument. Then you unecessarily shallow-copy it which is not a cheap operation. You are shifting the elements in the array onto a new memory address. Than you use table.concat which uses concatenation under the hood which ruins the point of your function.

And you are contradicting yourself. if the GC clears the memory in your concatStrings function after it terminates, the same applies when making a new string object when concatenating the normal way.

To be concise, your method is a lot more unperformant and expensive than normal concatenation and even uses normal concatenation under the hood.

No point in mentioning that since @blue1010123 is trying to yield for much longer than a frame (0.5 seconds). But yes, Heartbeat is more consistent than default RBLX wait because of Roblox’s poorly made task scheduler.

A good approach to a custom wait would be like so:

local Heartbeat = game:GetService('RunService').Heartbeat
local function Poll(T)
	local StartTime = os.clock() + T
	while true do
		if os.clock() >= StartTime then break end
		Heartbeat:Wait()
	end
end

My main point here is that you don’t need to run the function code on heartbeat. Function calls are managed in the call stack. Whenever you call a function, a stack frame is pushed onto the stack consisting of the return address, all function arguments, memory for any local variables, etc… Imagine doing that per heartbeat.

The amount of calls the call stack can handle really depends on the stack size. Here’s my source: https://www.learncpp.com/cpp-tutorial/the-stack-and-the-heap/

Back on topic, even if you decided to shove that function code in the loop, there would still be no point in checking the hours, minutes, and seconds per heartbeat frame since @blue1010123 is using math.floor to round meaning we don’t need such precision. Less checks means more performance and is still just as accurate.

local Heartbeat = game:GetService('RunService').Heartbeat
local function Poll(T)
	local StartTime = os.clock() + T
	while true do
		if os.clock() >= StartTime then break end
		Heartbeat:Wait()
	end
end
while true do
     -- Function Code
    Poll(0.5)
end

Don’t take micro-optimizations seriously. More serious optimizations such as using asymptotic analysis to reduce time complexity from O(n^2) to O(n) is a lot more major than deciding which is 0.00001 + faster. This is just one out of many good methods to optimizing code.

1 Like

I even did a benchmark here:

local function concatStrings(array)
	local t = {}
	for _, v in ipairs(array) do
		t[#t+1] = tostring(v)
	end
	return table.concat(t,"")
end

for i = 1, 100 do
	local FuncTime = nil
	local NormalTime = nil
	
	FuncTime = os.clock()
	local _ = concatStrings({"What ", "a ", "wonderful ", "world!"})
	FuncTime = os.clock() - FuncTime
	
	NormalTime = os.clock()
	local _ = "What ".."a ".."wonderful ".."world!"
	NormalTime = os.clock() - NormalTime
	
	print(( FuncTime < NormalTime and 'FuncTime is faster' ) or 'NormalTime is faster')
end

Normal concatenation was faster than your function for all 100 iterations and was consistently 10x faster.

@DarthFS I understand you are trying to contribute to this topic, and notice you make some good points, but please read my posts more thoroughly before negatively criticizing. I don’t mean to be impolite, but I see you didn’t always completely discern what I was trying to say.

Concatenation

I strongly disagree here. This function is far from hideous and is actually much better. Not really in this case as it turns out, however

7 - 8 individual strings long text (our case) :
--[[  ]]  ==> [string concatenation using dots was less than 5 times faster]

17+ individual strings long text (more demanding case) :
--[[  ]]  ==> [table concatenation was faster in every attempt]

2 * 10^5 individual strings long text

--[[ ]] ==> [table concatenation was more than 110 times faster]

I don’t appreciate my code being called hideous, especially because I don’t post code that I know doesn’t perform well (if I do by some small chance, I warn about it in advance). The run time is aproximately the same and it was just a solid idea (regarding micro-optimization, memory allocation and code space).

How did I test run time?

I repeated each test 5 times and removed the two most deviating results. I then compared the average.

Code 1
local NUMBER_OF_STRINGS = 10

local array = {}
local char, t, s

for i = 1, NUMBER_OF_STRINGS do
	char = math.random(97, 122)
	table.insert(array, string.char(char))
end

-- STRING CONCATENATION --------------
local tableTime, dotTime

dotTime = os.clock()
s = ""
for _, v in ipairs(array) do
	s = s .. v
end
dotTime = os.clock() - dotTime
--------------------------------------

-- TABLE CONCATENATION ---------------
tableTime = os.clock()
t = {}
for _, v in ipairs(array) do
	t[#t+1] = tostring(v)
end
t = table.concat(t, "")
tableTime = os.clock() - tableTime
--------------------------------------
--------------------------------------	
print(tableTime, dotTime)
print(( tableTime < dotTime and 'Table concat. method was faster.' ) or 'String concat. method was faster.')
Code 2
for i = 1, 500 do
	local strTime_sing, strTime_mult, tableTime, t, s, txt

	tableTime = os.clock()
	t = {}
	for _, v in ipairs({"This ", "is ", "a ", "long ", "sample ", "string", "!", " =)"}) do
		t[#t+1] = tostring(v)
	end
	t = table.concat(t, "")
	tableTime = os.clock() - tableTime

	strTime_sing = os.clock()
	txt = "This ".."is ".."a ".."long ".."sample ".."string".."!".." =)"
	strTime_sing = os.clock() - strTime_sing
	
	strTime_mult = os.clock()
	s = ""
	for _, v in ipairs({"This ", "is ", "a ", "long ", "sample ", "string", "!", " =)"}) do
		s = s .. v
	end
	strTime_mult = os.clock() - strTime_mult
	
	print("Single line:".. strTime_sing ..", multiple line".. strTime_mult .. ", table".. tableTime)
	
	local min = math.min(strTime_sing, strTime_mult, tableTime)
	print(
		min == strTime_sing and "single string concat" or
		min == strTime_mult and "multiple string concat" or
		"table concat"
	)
end

Run time is not actually the subject of focus here, but rather memory allocation is. The fact is that table concatenation requires more code, but less memory.

Concatenating in one line performs best with less strings while still being perforamnce friendly. I don’t see a reason why we are complicating everything so much.

I just offered some insight on how the function could be improved, and the change didn’t even take more than 1 minute. Time is not lost and @blue1010123’s code is improved. They are the one to decide whether to keep the changes or not according to what they are working on.

Because the topic is code review, various code improvement aspects and ideas are usually welcome.

Your benchmarking

While it is true that using arrays as arguments in functions is not a good idea, because it increases the possiblity of stack overflow happening, you could have modified the code a little bit before benchmarking. Another important aspect is function calling. Like you mentioned before, when function is called, it is pushed on a stack, and that takes an insignificant amount of time, but still relevant when doing benchmarks like this.

Your benchmarking is a good idea, but the results are not valid. When we avoid calling the function, we figure out that run time difference is almost negligible, far from order of magnitude.

Table concatenation method is neither really less performant nor more expensive compared to string concatenation. When it comes to more strings to be combined, it’s vice versa, and our case is relatively close to that border, since we are concatenating more than 7 strings in worst case.

Memory clearing

Despite this being correct, table methods requires less memory in general. String concatenating often requires more space to be allocated, although, not much in this case.

Heartbeat

Yes, there is a point, and I explained it already. If @blue1010123 ever decided to display miliseconds, Heartbeat is usually the right choice. As for accuracy, it’s the same, completely accurate, no matter the waiting time. Even if we waited 4 seconds, or 19 seconds, etc. we would still get the same completely accurate result, because we are calculating time delta between starting os.clock() and current os.clock().

Again, I never said Heartbeat should be used in this case. In fact, 0.3 is about the right wait time.

1 Like

Idk if I’m missing something here, but normal string concatenation seems to be faster with my benchmark and the other 2 of your benchmarks even when disregarding the function calls.

I apologize for being rude. The issue with your method, is that we do need to put that table.concat code in a function incase we want to use your method in different areas in your script. My original benchmark took care of the worst case scenario.

Also, from a practical game development standpoint, you aren’t going to end up concatenating over 15 strings 99% of the time. In @blue1010123 's scenario, he was concatenating up to 2 strings at once. Since he was doing that multiple times you would defiantly need to put that concatenation code in a function to prevent redundancies and to follow the D.R.Y principle making it even slower.

I feel like you are over-complicating it much more than needed.

I’m a bit confused here. The table method still contains all the concatenated strings. It’s the same amount of bytes as the string method.

local Array = {'X', 'D'}
local String = 'X'..'D'

In your function though, you have 1 array with let’s say as an example, {‘X’, ‘D’}, then you copy that data onto another table which in total now, is double the amount of bytes.

But @blue1010123 wants to change the text once per second. Not every 4 or 19 seconds.

You are missing my point. Since you are yielding for shorter periods of time, you are doing more unnecessary checks and decreasing performance. The in-game text changes once per second, yet you are checking every heartbeat. Most of those checks are still going to result in the same text because of the math.floor rounding.

How about you ask any experienced developer on their opinion on your method versus normal string concatenation. The idea of incorporating a custom concatenation function just for a few bytes doesn’t seem worth it. It’s worse in every other aspect.

1 Like

I wasn’t claiming that using table concatenation is better in this case. The matter is about comparison between the three approaches and their performance. Personally, I would almost definitely choose one-line concatenation, or perhaps break it down into two strings. All good here.

The issue becomes more significant with more strings. Around 13 or 14 strings, table method and string concat. methods start to ravel for the first place. However, one-line method is not effective with a lot of strings, and neither is classical string concatenation. Why?

If we had 20 strings to be combined, it would be very impractical to concat. them all at once. What about gradual concatenation?

str = str .. new string here 

Doing this 20 times means creating 20 individual versions of that string. Bad for performance.

Compared to table concatenation, which is supposedly more performance friendly, similarly to one-line concat.

I tried to roughly measure performance spikes. String concatenation of 2,5 *10^5 strings caused an increase of used memory by more than 100 mb, as opposed to table concatenation, which required no more than a couple of megabytes.

Seems correct, but doesn’t account for gradual adding up. Turns out that tables are relatively friendly.

Why you didn’t get the same results? Because I was testing with various NUMBER_OF_STRINGS values. Turning point is never achieved using one-line concat, but is with gradual concatenating.

Imagine we have an infinite array of 4 characters long
strings.

Operation ::::: character num. stored altogether

" " .. "abcd" --> 4
"abcd" .. "abcd" --> (4 + 4) + 4 = 12
Total bye size increases this way. Tables concat them all at once.

Everything was alright originally, so I’m not sure how we opened the discussion about effectiveness of these three methods. :laughing: The idea was almost neglectable difference between one-line and multiple line concatenation. And the fact that excessive combining is not good. Return in one line seemed readable to me, so that’s why I added it. Why not? We are talking about full optimization of pasted code, so this is a micro step forward. If readability is drastically reduced, the reader can always skip that step in optimization.

About Heartbeat,

I meant:

Display with miliseconds

hours : minutes : seconds : miliseconds

00 : 00 : 00 : 00

We cannot use wait(0.3) time interval, because time 
difference is too big (300 ms). Thats when Heartbeat
comes in handy.

@DarthFS we obviously misunderstood each other a little. Hopefully, everything is clearified now. I also hope you learned something new as I certainly did, especially about concatenation and function calls. My posts sometimes include extra tips and aspects that may not play a key role, but can at least be of more informative nature, like the extra tip about Heartbeat. Not at all is this meant for use when displaying seconds only.

Have a good day and good luck with your projects!

os.clock() returns CPU time, which idiomatically should only be used during benchmarks. For more generic usecases, use time().

You are bringing up micro-optimization to (arguably) a beginner programmer, which is redundant and will only confuse them with convoluted terms. Also, micro-optimizing is only bad behavior in this case, because you will be pre-optimizing code that has absolutely no need to be micro-optimized - it would possibly just end up with the function breaking when trying to micro-optimize.

1 Like

@PysephDEV thank you for your reply too. I agree that one line concatenation is not necessary. However, code review Dev Forum section is intended for discussing different ways to improve code, so mentioning string concatenation from the performance aspect seems reasonable.

It’s not only about using the code. I was showed another path to concatenating, and anyone can freely ignore it. It is of informative nature.

Short-circuit evaluation isn’t necessary here, but is there anything wrong with mentioning it anyway? Still better than returning the code back, because there is almost nothing to improve. The original poster and other readers don’t learn much that way.

As for os.time() and os.clock(), the former is by definition indeed intended solely for benchmarking, but its role is much broader. tick() is deprecated, so os.clock() is the only reliable precise function. Since we are working in seconds, time() seems sufficient, although there is, to my knowledge, nothing wrong with using os.clock(). We are not formating time, but calculating time delta. I’m more inclined on using os.clock() for miliseconds.

Why did I suggest os.clock(). It’s, again, precise. But there is otherwise no real reason. os.clock() was always known as very reliable and delay free. It has often been my first choice, and there is no (at least not easily accessible) documentation suggesting not using os.clock() generally.

os.time() and os.clock() have practically the same call time.

My first post has now been updated a lot. It’s perhaps a better reading material and this briefly summarizing reply.

Wish you a pleasant day!

I specifically implied time(), not os.time(). These are very different functions. Yet again os.clock() is idiomatically not meant to be used for anything outside of benchmarks, and doing otherwise would be bad practice. You can see on the wiki itself that it says “[…] and is intended for use in benchmarking.” I don’t know how else you want this to be elaborated to you - the wiki is already being concise about it. Putting it this way: You are recommending someone to kill a duck using a knife - sure, you yourself probably use it just fine and like using it, but to others, they would much rather prefer a tool that is tailored specifically for this type of job, I.e. a gun. No one is telling you that you can’t use os.clock outside of it’s intended range, but that would not be following idiomatic practices.
Also, if you read the rules of the section it clearly says not to delve into micro-optimization, which you have clearly done. Not only so, but you focused most of your reply on it, too.