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

Fix #2937: Improve performance of tocolor #3044

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

TracerDS
Copy link
Contributor

@TracerDS TracerDS commented May 25, 2023

Fixes #2937:
Moved tocolor into an embedded section. That way it should be faster...?

@TracerDS TracerDS marked this pull request as ready for review May 25, 2023 08:59
@xLive
Copy link
Member

xLive commented May 25, 2023

Did you perform any benchmarks? The assert and range checks will make it significantly slower than the C++ implementation.

@TracerDS
Copy link
Contributor Author

Did you perform any benchmarks? The assert and range checks will make it significantly slower than the C++ implementation.

Unfortunatelly I did not. Didnt have the chance.
In that case a simple if statement may be used instead

@lopezloo lopezloo added the enhancement New feature or request label May 25, 2023
@ArranTuna
Copy link
Collaborator

crun t1 = getTickCount() for i=1, 1000000 do tocolor(1, 2, 3, 4) end t2 = getTickCount() outputChatBox(t2-t1)
254

I then added your lua version of tocolor into runcode and it took 444 ms so this proves that this does NOT increase performance and actually degrades performance? As well as the fact that if you have 500 resources, each lua VM has a copy of this tocolor, so it's just unnecessarily increasing memory usage of every resource.

@patrikjuvonen patrikjuvonen added the feedback Further information is requested label Jun 12, 2023
@patrikjuvonen patrikjuvonen changed the title Improve performance of tocolor - #2937 Fix #2937: Improve performance of tocolor Jun 12, 2023
@patrikjuvonen patrikjuvonen marked this pull request as draft June 12, 2023 11:08
@TracerDS
Copy link
Contributor Author

TracerDS commented Jun 12, 2023

crun t1 = getTickCount() for i=1, 1000000 do tocolor(1, 2, 3, 4) end t2 = getTickCount() outputChatBox(t2-t1) 254

I then added your lua version of tocolor into runcode and it took 444 ms so this proves that this does NOT increase performance and actually degrades performance?

Hmm, it looks like type() function is significantly slowing everything. But without that an error might occur 🤔

As well as the fact that if you have 500 resources, each lua VM has a copy of this tocolor, so it's just unnecessarily increasing memory usage of every resource.

This is not a significant increase in memory, if any

@thisdp
Copy link
Contributor

thisdp commented Jun 17, 2023

crun t1 = getTickCount() for i=1, 1000000 do tocolor(1, 2, 3, 4) end t2 = getTickCount() outputChatBox(t2-t1) 254
I then added your lua version of tocolor into runcode and it took 444 ms so this proves that this does NOT increase performance and actually degrades performance?

Hmm, it looks like type() function is significantly slowing everything. But without that an error might occur 🤔

As well as the fact that if you have 500 resources, each lua VM has a copy of this tocolor, so it's just unnecessarily increasing memory usage of every resource.

This is not a significant increase in memory, if any

It's nothing about type(). The main reason is function call in lua is extremely slow. That's why i said lua implemented tocolor will decrease performance.

And another problem, your code can not work if r g b a are float like 128.5

@CrosRoad95
Copy link
Contributor

maybe we should focus on fixing "core issue" - you use "tocolor" because "dxDraw" functions need to be called every frame, what if, we design some "widget system" where you declare it, example rectangle, set color once and mta automatically drawing it?

local rect = dxCreateRectangleWidget(100, 100, 100, 100, tocolor(255, 0, 0)) -- no "onClientRender" required

it will be equivalent of onClientRender + dxDrawRectangle every frame
with functions such a: change position, size, color ect

@thisdp
Copy link
Contributor

thisdp commented Jun 17, 2023

maybe we should focus on fixing "core issue" - you use "tocolor" because "dxDraw" functions need to be called every frame, what if, we design some "widget system" where you declare it, example rectangle, set color once and mta automatically drawing it?

local rect = dxCreateRectangleWidget(100, 100, 100, 100, tocolor(255, 0, 0)) -- no "onClientRender" required

it will be equivalent of onClientRender + dxDrawRectangle every frame with functions such a: change position, size, color ect

btw, tocolor is faster enough, dx functions are far more slower

@thisdp
Copy link
Contributor

thisdp commented Jun 17, 2023

maybe we should focus on fixing "core issue" - you use "tocolor" because "dxDraw" functions need to be called every frame, what if, we design some "widget system" where you declare it, example rectangle, set color once and mta automatically drawing it?

local rect = dxCreateRectangleWidget(100, 100, 100, 100, tocolor(255, 0, 0)) -- no "onClientRender" required

it will be equivalent of onClientRender + dxDrawRectangle every frame with functions such a: change position, size, color ect

maybe also need a
dxSetRectangleWidgetConfig(widget,100, 100, 100, 100, tocolor(255, 0, 0))

widget is a number instead of element

@ArranTuna
Copy link
Collaborator

There's actually a really simple fix for this which I already use except in the few cases where the color frequently changes instead of:

dxDrawText(blah, tocolor(255, 255, 255, 255), blah)

You just add this at top of script:

local white = tocolor(255, 255, 255, 255)

Then:

dxDrawText(blah, white, blah)

@Pirulax
Copy link
Contributor

Pirulax commented Jun 25, 2023

or, just learn to use hex instead of tocolor?

tocolor(255, 255, 255, 255) == 0xFFFFFFFF

The hex format is 0xAARRGGBB
FF is 255, 7F would be 127, and so on.. You'll learn it the more you use it.

@thisdp
Copy link
Contributor

thisdp commented Jun 25, 2023

or, just learn to use hex instead of tocolor?

tocolor(255, 255, 255, 255) == 0xFFFFFFFF

The hex format is 0xAARRGGBB FF is 255, 7F would be 127, and so on.. You'll learn it the more you use it.

Best way to increase performance of tocolor (Deprecate tocolor) :merow:

@dmi7ry
Copy link

dmi7ry commented Jun 25, 2023

The hex format is 0xAARRGGBB FF is 255, 7F would be 127, and so on.. You'll learn it the more you use it.

But things get noticeably worse when you need to change individual components dynamically. Native implementation is better for such cases.

@TracerDS TracerDS marked this pull request as ready for review September 24, 2023 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request feedback Further information is requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve performance of tocolor
9 participants