Skip to content

Performance improvements of some lua APIs#99

Open
Helyos96 wants to merge 1 commit intoPathOfBuildingCommunity:masterfrom
Helyos96:performance-improvements
Open

Performance improvements of some lua APIs#99
Helyos96 wants to merge 1 commit intoPathOfBuildingCommunity:masterfrom
Helyos96:performance-improvements

Conversation

@Helyos96
Copy link
Copy Markdown

  • Use lua_tonumberx/lua_tointegerx instead of separated calls like lua_isnumber/lua_tonumber
  • Avoid evaluating luaL_typename in the asserts when there is no need (these were always evaluated even when the assert didn't trigger)
  • Drop the sscanf() call in ReadColorEscape
  • Avoid calling IsColorEscape twice for ReadColorEscape

While profiling SimpleGraphics, I saw that l_DrawImage, l_DrawImageQuad and l_SetDrawColor were very consuming while in tree view (not very surprising).

I found a few improvements to do, which mostly revolve around eliminating some calls to luaJit and dropping a costly sscanf().

Profiling before:

image

And after:

image

The most obvious gain is l_SetDrawColor with sscanf being dropped. The l_DrawImage functions saw fewer gains but still less time spent in lua functions, especially the calls to lua_type went much lower.

Looking at task manager, PoB went from ~6.5% (maxing out one logical core) to ~5.9%. It could be that in doing so I gained a few fps, which skewed a bit the profiling data since these functions got to be called more.

* Use lua_tonumberx/lua_tointegerx instead of separated calls like
  lua_isnumber/lua_tonumber
* Avoid evaluating luaL_typename in the asserts when there is no need
  (these were always evaluated even when the assert didn't trigger)
* Drop the sscanf() call in ReadColorEscape
* Avoid calling IsColorEscape twice for ReadColorEscape
Copy link
Copy Markdown
Contributor

@zao zao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The transformations look valid from reading the code and it builds locally.
I didn't run any performance tests but the reasoning here seems sound.

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.

2 participants