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 Vector:LengthSqr() instead of Vector:Length() #28

Open
someoneyouspite opened this issue Jan 11, 2021 · 3 comments
Open

Use Vector:LengthSqr() instead of Vector:Length() #28

someoneyouspite opened this issue Jan 11, 2021 · 3 comments

Comments

@someoneyouspite
Copy link
Contributor

This would be more efficient than calculating a square root to compare against.

imgui/imgui.lua

Line 115 in 9219cb2

local distance = eyePosToPos:Length()

@wyozi
Copy link
Contributor

wyozi commented Jan 12, 2021

Can you (or anyone) provide a benchmark proving that it would be more efficient?

Squared distance being so much more efficient than normal distance that it should be changed everywhere is probably the biggest misunderstanding around gmod devs. I wouldn't necessarily be against changing it here if done properly, since it wouldn't affect code complexity that much, but oh boy does everyone like suggesting this "optimization" without actually testing if it has any impact first.

@TomDotBat
Copy link
Contributor

The benchmark below shows a very minimal improvement, however the execution time of the square root calculation ultimately depends on the input given.

local benchmark
local vec = VectorRand() * 1000
local getLength = vec.Length
local getLengthSqr = vec.LengthSqr

print("Benchmark data : ", vec, "\n")

do
    benchmark = SysTime()

    for i = 1, 2e5 do end

    print("Baseline : ", SysTime() - benchmark)
end


do
    benchmark = SysTime()

    for i = 1, 2e5 do
        getLength(vec)
    end

    print("Vector:Length() : ", SysTime() - benchmark)
end


do
    benchmark = SysTime()

    for i = 1, 2e5 do
        getLengthSqr(vec)
    end

    print("Vector:LengthSqr() : ", SysTime() - benchmark)
end

print("\n")

Results (after 5 runs):

Benchmark data : 	495.429901 739.004211 436.868652	

Baseline : 	9.6400000074937e-05
Vector:Length() : 	0.0059317999998711
Vector:LengthSqr() : 	0.0055161000000226


Benchmark data : 	-522.012939 -765.965698 -75.552155	

Baseline : 	8.6899999814705e-05
Vector:Length() : 	0.0058865999999398
Vector:LengthSqr() : 	0.0055757000000085


Benchmark data : 	256.878876 818.915649 -309.296326	

Baseline : 	0.0001259000000573
Vector:Length() : 	0.0059771999999612
Vector:LengthSqr() : 	0.0057152000001679


Benchmark data : 	375.659027 -366.963501 -569.133728	

Baseline : 	9.0699999873323e-05
Vector:Length() : 	0.0060378999999102
Vector:LengthSqr() : 	0.0056388000000425


Benchmark data : 	-381.458496 -671.425659 715.860413	

Baseline : 	8.87999999577e-05
Vector:Length() : 	0.0059182999998484
Vector:LengthSqr() : 	0.0056335000001582

@wyozi
Copy link
Contributor

wyozi commented Jun 12, 2021

Thanks! So ~5% improvement (or very roughly 10000th of a millisecond per call).

I wouldn't mind the change here, since it's very localized and requires only two lines changed. It is quite a micro-optimization, however. I guess crossing the C++ boundary is the source of slowness here.

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

No branches or pull requests

3 participants