search

ec- / Quake3e

Improved Quake III Arena engine
GNU General Public License v2.0
1.19k stars 154 forks source link

Optimize BoxOnPlaneSide function #229

Closed LegendaryGuard closed 1 year ago

LegendaryGuard commented 1 year ago

Reference to https://github.com/ec-/baseq3a/pull/27

The inline assembly version of this function can only be compiled on MSVC (Microsoft Visual C++).

ghost commented 1 year ago

Somehow your code changes look familiar to me, did you take these changes from Xreal? If so, it would be nice to mention this somewhere, anyways in Xreal this code works well.

LegendaryGuard commented 1 year ago

Somehow your code changes look familiar to me, did you take these changes from Xreal? If so, it would be nice to mention this somewhere, anyways in Xreal this code works well.

Nope, the code was from Quake III Arena Kenny Edition, using the optimized ones, and I did some tweaks.

ghost commented 1 year ago

Ah, okay! Just for the record I think it was first used here: https://sourceforge.net/p/xreal/svn/427/ Maybe this will help you to find the fastest and most reliable approach, though maybe Quake III Arena Kenny Edition took it from Xreal and optimized it already as well.

LegendaryGuard commented 1 year ago

Ah, okay! Just for the record I think it was first used here: https://sourceforge.net/p/xreal/svn/427/ Maybe this will help you to find the fastest and most reliable approach, though maybe Quake III Arena Kenny Edition took it from Xreal and optimized it already as well.

Ok, changed the based reference on the comment. XreaL is the original and the origin of this reference.

ghost commented 1 year ago

Sorry for the slightly off topic question: Do you know any procedure to test those math optimizations? Do you use any tools to test the 'optimization'? I also often try to use 'faster' math, but I don't really can see any improvement. Thanks!

ec- commented 1 year ago

This x87 code is not better than current C version output generated by modern compilers

LegendaryGuard commented 1 year ago

This x87 code is not better than current C version output generated by modern compilers

So, gotta remove this inline assembly, I was looking info about that, I found recommendations that inline assembly on MSVC shouldn't be used (because it can hurt the performance a bit).

Sorry for the slightly off topic question: Do you know any procedure to test those math optimizations? Do you use any tools to test the 'optimization'? I also often try to use 'faster' math, but I don't really can see any improvement. Thanks!

Researching, learning the logic, problem resolution, ... Not sure about the tools to test the optimization, Microsoft Visual Studio Community has a lot of tools to see a list of warnings, the quantity of lines of code, profiler, CPU usage, memory usage, ... Task manager looks like a profiler too, you can see the CPU and memory usage of the process. I don't know how faster can be something, the C code part I just checked is the most optimized compared to the previous one (the one that has a for loop).

LegendaryGuard commented 1 year ago

Ok, when you agree and are ready to merge, squash commits before merging.

ec- commented 1 year ago

Ok, when you agree and are ready to merge, squash commits before merging.

You basically just reverted to original 1.32b C version and added // That code was based on XreaL comment to it, there is nothing to merge