mtasa-blue icon indicating copy to clipboard operation
mtasa-blue copied to clipboard

Implement lua-vec - vectors as native Lua datatype

Open Pirulax opened this issue 3 years ago • 20 comments

This PR aims to fix the issue we have with userdata vectors. (#321) As discussed on discord (and maybe even in the issue itself), using Lua tables would be only marginally better, and the performance benefit would probably be lost when doing table <=> conversion. Note: I've tested this with bytecode as well (Compiled all default resources, and they all worked perfectly). So no backwards compatibility issues here.

This code(Lua-vec) was "stolen" from here.

I need opinions: Although I've already discussed this with @Woovie, I'd be great if others could give their feedback. I think naming the datatype as vector would be totally okay (the function table will be named vector in lower case, just so we keep Lua's naming convention. We can just alias it to Vector in a form of a global variable)

Features in lua-vec:

  • Nice indexing of vectors, either with indecies, .x, or even .xxyz, .xx (like in hlsl)
  • Really fast (even compared to Lua table implementation). from 4x up to 10x faster than Vector4.
  • Very memory friendly. Our current Vector implementation is a memory hog([1]). Lua-vec uses no additional memory at all! Its free! [2]

[1]: A Vector4 needs 4 bytes for lua_newuserdata, another sizeof(unsigned long) + sizeof(void*) + sizeof(CLuaVector4D (which is 16 + 4 bytes) in CIdArray [2]: Its value is stored in a GcObject union. This is where the userdata is stored as well, so, compared to the userdata implementation, we use no additional memory

Todo:

  • Make vec callable (add __call metamethod), right now vectors can only be created with vec.new
  • Make it possible for ArgumentParser and CScriptArgReader to read the new type as a Vector2/3/4D
  • Add support to construct Vector2/3/4 from native vector
  • Add support to construct native vector from existing Vector2/3/4 (only way to implement this cleanly is to use .x .y .z)
  • @qaisjp suggested that it would be nice to have a separate branch to keep track of our modifications to Lua.
  • Implement LUA_TVEC in CLuaArgument (WriteToBitStream and ReadFromBitStream)

Tests: Resource used: vectest.zip Scene rendering with smallpt.lua (from Lua-vec):

Command: runsmallpttests 250 250 4 false

Settings: w: 250, h: 250, samples: 4, do_force_gc_collect: false
============[vec]============
Writing to file... took 176 ticks
Tracing took 115081 ticks
Memory used: 3.3 gb

============[stdlua]============
Writing to file... took 291 ticks
Tracing took 371085 ticks
Memory used: <= 3.3 gb

============[Vector4]============
Ran out of memory. Needs around 16 gigs.

Command: `runsmallpttests 250 250 4 true`
Settings: w: 250, h: 250, samples: 4, do_force_gc_collect: true
============[vec]============
Collect garbage total: 19830 ticks
Writing to file... took 165 ticks
Tracing took 103906 ticks

============[Smallpt - Vector4]============
Collect garbage: 25859 ticks
Writing to file: 338 ticks
Tracing: 475007 ticks

============[Smallpt - stdlua]============
Collect garbage: 15044 ticks
Writing to file: 344 ticks
Tracing: 360707 ticks

Syntethic add/create/mul/sub benchmark:

Iterations: 10000000
============[native-vector - GC: OFF]=======
=> create: 2694 ms
=> sub: 1692 ms
=> mul: 1681 ms
=> eg: 535 ms
=> add: 1716 ms
============[native-vector - GC: ON]========
=> create: 2228 ms
=> sub: 1338 ms
=> mul: 1313 ms
=> eg: 507 ms
=> add: 1327 ms
===========================================

============[Vector2 - GC: OFF]============
=> create: 9966 ms
=> sub: 10291 ms
=> mul: 10085 ms
=> eg: 2138 ms
=> add: 9660 ms
============[Vector2 - GC: ON]============
=> create: 30965 ms
=> sub: 21562 ms
=> mul: 21275 ms
=> eg: 5635 ms
=> add: 17737 ms
===========================================


============[Vector3 - GC: OFF]============
=> create: 22923 ms
=> sub: 11093 ms
=> mul: 12490 ms
=> eg: 2196 ms
=> add: 13755 ms
============[Vector3 - GC: ON]============
=> create: 47758 ms
=> sub: 15084 ms
=> mul: 15346 ms
=> eg: 4736 ms
=> add: 13413 ms
===========================================

============[Vector4 - GC: OFF]============
=> create: 14349 ms
=> sub: 31588 ms
=> mul: 24745 ms
=> eg: 2147 ms
=> add: 12531 ms
============[Vector4 - GC: ON]============
=> create: 50227 ms
=> sub: 16102 ms
=> mul: 15704 ms
=> eg: 4504 ms
=> add: 13207 ms
===========================================

Pirulax avatar Oct 17 '20 15:10 Pirulax

So, whats people's opinion on this? I'd be really happy to see this in 1.6. Also it seems like we could without problems add support for matrices as well. A GCObject is 180 bytes, a regular 3x3 matrix is.. 48, which means that the unions size would stay the same.

Pirulax avatar Nov 13 '20 05:11 Pirulax

I like the changes.

@qaisjp suggested that it would be nice to have a separate branch to keep track of our modifications to Lua.

We can have a submodule for Lua. In this way, it will be easy to track the changes.

ghost avatar Dec 09 '20 10:12 ghost

So, we should create a separate repo?

Pirulax avatar Dec 09 '20 13:12 Pirulax

So, we should create a separate repo?

Hmm, that doesn't seem like a good idea. Let's just create a new branch for Lua. I'd do it like this:

  1. Download Lua 5.1 original source and commit the code to the branch.
  2. Copy the Lua source from MTA vendor to the new branch.
  3. And then create a PR for lua-vec changes.

ghost avatar Dec 09 '20 15:12 ghost

My approach would be:

  1. Fork Lua repo to multitheftauto org. This keeps original history instead of destroying the Lua 5.1 history, and makes it easier to cherry-pick improvements later.
  2. Cherry-pick our custom 5.1 mods from mtasa-blue to a branch on the Lua repo
  3. Replace vendor/lua in mtasa-blue with a submodule that targets that Lua repo
  4. Create a PR for lua-vec changes in the Lua repo.

This approach is compatible with the submodule system proposed in #319.

qaisjp avatar Dec 09 '20 17:12 qaisjp

AFAIK There is no official Lua repo on GitHub, if that's what you meant by Fork Lua repo. I dont think the Lua history is necessary. I'll just create a new repo with the 5.1 source code from Lua website

Pirulax avatar Mar 08 '21 14:03 Pirulax

Okay I think I did it. Is this what you meant?

Pirulax avatar Mar 08 '21 15:03 Pirulax

AFAIK There is no official Lua repo on GitHub, if that's what you meant by Fork Lua repo. I dont think the Lua history is necessary. I'll just create a new repo with the 5.1 source code from Lua website

There's an official mirror on GitHub of the Lua repo, here. It has a tag for every released Lua version, all the way back to 5.1. I agree that keeping the history may come in handy in the future for some cherry-picks, for updating Lua more easily, or whatever, but I would also understand not keeping the original commit history if it's deemed too verbose.

AlexTMjugador avatar Mar 13 '21 11:03 AlexTMjugador

Please look at MTA org's /lua repo. I have filtered our commits to vendor/lua + added base Lua at the top (source code for 5.1.5 downloaded from Lua website). Would that fit?

Pirulax avatar Mar 14 '21 20:03 Pirulax

i noticed you call the vector type in Lua only "vec" which is inconsistent to the other types: "number", "table", ... the type should be "vector"

LosFaul avatar Mar 16 '21 20:03 LosFaul

since Lua uses doubles, shouldn't the vectors not use doubles too?

LosFaul avatar Mar 16 '21 20:03 LosFaul

There would be no benefit of the additional memory required. MTA uses float vectors everywhere, the additiona precision would be lost. Yeah, the type name will be changed, once this PR gets reviewed or something.. Tbh, we didnt really agree on a name yet. I think vector (perhaps with a capital V) is acceptable. Since it's not vanilla Lua, we might as well stick to our naming convention.

Pirulax avatar Mar 16 '21 20:03 Pirulax

maybe cVector where c stands for custom or if someone has better idea he should write it

LosFaul avatar Mar 17 '21 11:03 LosFaul

btw. since my knowledge is not too big about instruction sets so, only asking if it's possible: could it be possible that the vectors could somehow utilize something like sse2 for even higher performance?

LosFaul avatar Mar 17 '21 11:03 LosFaul

Please look at MTA org's /lua repo. I have filtered our commits to vendor/lua + added base Lua at the top (source code for 5.1.5 downloaded from Lua website). Would that fit?

I'm not sure if you were talking to me, but yeah, it looks good to me. I was just pointing out that the original approach that @qaisjp suggested was possible due to the existence of that mirror.

AlexTMjugador avatar Mar 17 '21 12:03 AlexTMjugador

Anyways, the submodule stuff #2112

Pirulax avatar Mar 17 '21 13:03 Pirulax

The main bottleneck is probably still the Lua side, rather than the actual maths. Not worth the headache. But where I think we might benefit from SSE is our vector stuff.. although I wonder why MSVC doesn't use SSE by default there.. Should be pretty trivial I belive.

Pirulax avatar Mar 17 '21 13:03 Pirulax

Edit: Turns out there's 2 lua-vec branches: One which is a first-class value like a number, bool, etc.. and the second one (this) treats it as GCObject's. Obviously the first one is faster, but comes at the expense of making TValue be 16 bytes instead of 8 (Which translates to basically 2x memory usage for Lua values) and probably performance loss as it would take 4 memory fetches to move the value into the register. The GC'd version is probably already fast enough IMHO. Although it uses some hackery (some kind of free list?).

Pirulax avatar Mar 23 '21 21:03 Pirulax

@Pirulax A great amount of time has passed, but do you still want to get this pull request merged?

botder avatar Jul 25 '23 14:07 botder

This is very useful and would be a good update. You don't plan to continue dealing with this PR in the future?

Nico8340 avatar Apr 27 '24 04:04 Nico8340