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

Use the faster table insertion method (dlg_settings_advanced.lua only) #3823

Closed
wants to merge 1 commit into from
Closed

Conversation

ghost
Copy link

@ghost ghost commented Mar 7, 2016

No description provided.

@sofar
Copy link
Contributor

sofar commented Mar 8, 2016

Not really critical or anything hotpath, but still fine I think.

@ghost
Copy link
Author

ghost commented Mar 8, 2016

Not really critical or anything hotpath

Yes. But even not critical, I think the faster the better.

@kilbith
Copy link
Contributor

kilbith commented Mar 8, 2016

On non-critical code you need readability and ease of maintenance more than a micro-optimization.

There are other ways to make the insertion faster, but it would be dumb to lose the readability for 3 ms. faster here...

@paramat
Copy link
Contributor

paramat commented Mar 9, 2016

👍

@kaeza
Copy link
Contributor

kaeza commented Mar 11, 2016

tinsert(t,v)      2365273
t[#t+1]=v         2522097
n=n+1 t[n]=v      7909498

The numbers are insertions per second using different approaches (tested under LuaJit, stock Lua may be different; benchmark here if you want to test).

Still agree with @kilbith.

@sofar
Copy link
Contributor

sofar commented Mar 11, 2016

lua-5.1.2, tested with the pasted link by kaeza:

DESC              TOT
tinsert(t,v)      1938457.3333333
t[#t+1]=v         2280419.3333333
n=n+1 t[n]=v      3821620.6666667

@sofar
Copy link
Contributor

sofar commented Mar 11, 2016

For reference, here is a full spread of test data across versions:

Numbers are insertions per second

=== lua-5.1.4 ====
DESC              TOT
tinsert(t,v)      1894375.3333333
t[#t+1]=v         2231591.6666667
n=n+1 t[n]=v      3757611
=== lua-5.2.3 ====
DESC              TOT
tinsert(t,v)      1820607.6666667
t[#t+1]=v         2076934.3333333
n=n+1 t[n]=v      3404882.3333333
=== lua-5.3.2 ====
DESC              TOT
tinsert(t,v)      1957116.6666667
t[#t+1]=v         2088806.6666667
n=n+1 t[n]=v      3481470.6666667
=== luajit ====
DESC              TOT
tinsert(t,v)      2392461
t[#t+1]=v         2597967
n=n+1 t[n]=v      4398963.3333333

My biased opinion:

  • It's nonsense to convert from tinsert() to t[#t + 1], it's maybe 5% difference
  • Incrementing a table index is way, way faster (50%)

@ghost
Copy link
Author

ghost commented Mar 12, 2016

I will close this PR and revert 24e8b0a with new PR.

@ghost ghost closed this Mar 12, 2016
@ghost ghost mentioned this pull request Mar 19, 2016
@ghost ghost deleted the patch-1 branch March 19, 2016 12:20
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants