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

Faster insertion into table #3448

Closed
wants to merge 1 commit into from
Closed

Faster insertion into table #3448

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Dec 12, 2015

minetest/minetest_game#732

-- table.insert
local time1 = os.clock()
local testtable1 = {}
for i = 1, 1000 do
    table.insert(testtable1, i)
end
print(os.clock()-time1)

-- tbl[#tbl+1]
local time2 = os.clock()
local testtable2 = {}
for i = 1, 1000 do
    testtable2[#testtable2+1] = i
end
print(os.clock()-time2)

table.insert :0.00020699999999996
tbl[#tbl+1] :0.00011100000000003

@ghost
Copy link
Author

ghost commented Dec 12, 2015

👍

@PilzAdam
Copy link
Contributor

What about Lua vs. LuaJIT? Is it better for both?

Also you probably shouldn't waste your time optimizing non performance critical code.

@ShadowNinja
Copy link
Member

My timings (with iterations multiplied by 1000 for better sample size):

Lua 5.3.2:
0.266029
0.16961
LuaJIT 2.0.4:
0.141926
0.126934

This doesn't matter unless the code is very performance-critical though.

@est31
Copy link
Contributor

est31 commented Dec 12, 2015

We don't use 5.3 we use 5.1.

@ShadowNinja
Copy link
Member

@est31: Yes, but I don't have Lua 5.1 installed, I'd have to turn the script into a mod and run it through Minetest by starting a world.

@sapier
Copy link
Contributor

sapier commented Dec 13, 2015

doesn't seem to cause side effects on non indexed (key based) tables too

@sapier
Copy link
Contributor

sapier commented Dec 29, 2015

come on guys this is a minor change can those denying it please explain why?

@est31
Copy link
Contributor

est31 commented Dec 29, 2015

From reading http://www.lua.org/manual/5.1/manual.html#2.5.5 I saw that #tablename +1 is guaranteed to be nil. thats very fine

👍 if the whitespace issue (tablename[#tablename+1]) is fixed (should be tablename[#tablename + 1]).

@ghost
Copy link
Author

ghost commented Dec 30, 2015

Updated.

@est31
Copy link
Contributor

est31 commented Dec 30, 2015

👍 looks good

@ghost
Copy link
Author

ghost commented Mar 6, 2016

Rebased.

@paramat
Copy link
Contributor

paramat commented Mar 6, 2016

👍 Will merge soon.

@paramat
Copy link
Contributor

paramat commented Mar 6, 2016

24e8b0a

@paramat paramat closed this Mar 6, 2016
@ghost ghost deleted the replace_table_insert branch March 7, 2016 06:31
@ghost ghost mentioned this pull request Mar 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants