Emit & Dyn_emit now wrap string.format + Vector instruction segment fix

[DerelictDrone]

May fix #26 for vector instructions, tested with VADD but needs further testing on the other VEXP instructions.

Implements @Vurv78's suggestion for ZVM:Emit() and ZVM:Dyn_Emit(), since they take strings, making them wrap string.format() to stop concatenating strings and numbers in a potentially unsafe manner.

Converts all known calls to dyn_emit and emit that concatenate numbers in zvm_opcodes and gmod_wire_cpu to use %d

Dyn_EmitOperand remains unaffected at the moment due to taking multiple args already

[thegrb93]

Looks like an improvement, but is a huge cost to performance as well. I'd like to see the 'Emit' shit replaced with actual lua rather than string-building into compiled lua code at some point. This is fine for now at least.

[DerelictDrone]

Looks like an improvement, but is a huge cost to performance as well. I'd like to see the 'Emit' shit replaced with actual lua rather than string-building into compiled lua code at some point. This is fine for now at least.

I have been wondering if there may be a performance improvement in reducing the number of calls to string format and global emit/dyn_emit, by possibly replacing them with a version that builds a smaller, local string(saving the arguments), then calling the global emit at the end which will run string.format once on the whole instruction's code rather than string.formatting and concatenating to the entire block on every emit/dyn_emit

I'd like to ask others that are more familiar with lua's performance regarding the local part, but I imagine less frequent (but bigger) calls to string.format would go a long way

[thegrb93]

Yeah the less concats and formats, the better.

[thegrb93]

You could potentially combine a bunch of those groups of emits into a single emit.

[Vurv78]

Looks like an improvement, but is a huge cost to performance as well. I'd like to see the 'Emit' shit replaced with actual lua rather than string-building into compiled lua code at some point. This is fine for now at least.

Didn't realize it ran at runtime. That is pretty terrible. Then again the emit function itself is very laggy just looking at the code. It uses repeated string concatenation so a string.format call is the least of our problems considering the huge amount of allocations this alone is causing.

If it were me I'd refactor it to store it properly as a string buffer and only table.concat when it's needed, and replace the varargs with parameters a-f so the code can jit compile.

[DerelictDrone]

Should change this emit function too, no?

https://github.com/wiremod/wire-cpu/blob/39b3a4b7e4af5a61e3a9b43662e0bb54fd602329/lua/wire/zvm/zvm_core.lua#L28

@Vurv78 That one can only occur if microcodedebug is on, the only condition that sets it is if not SERVER and not CLIENT, I don't know if such a state is possible unless you're running it outside of gmod, or manually assigning it

[DerelictDrone]

Looks like an improvement, but is a huge cost to performance as well. I'd like to see the 'Emit' shit replaced with actual lua rather than string-building into compiled lua code at some point. This is fine for now at least.

Didn't realize it ran at runtime. That is pretty terrible. Then again the emit function itself is very laggy just looking at the code. It uses repeated string concatenation so a string.format call is the least of our problems considering the huge amount of allocations this alone is causing.

I'm fairly certain these will only run on compile/precompile, I.E that section is only encountered for the first time and hasn't been modified by the CPU itself

I've noticed that you can tell easily when something like the GPU is starting a precompile by it flashing the "took too long to draw frame!" the first time you enter a new block.

If it were me I'd refactor it to store it properly as a string buffer and only table.concat when it's needed, and replace the varargs with parameters a-f so the code can jit compile.

If you think this'll help I can do it.

[Vurv78]

Should change this emit function too, no? https://github.com/wiremod/wire-cpu/blob/39b3a4b7e4af5a61e3a9b43662e0bb54fd602329/lua/wire/zvm/zvm_core.lua#L28

@Vurv78 That one can only occur if microcodedebug is on, the only condition that sets it is if not SERVER and not CLIENT, I don't know if such a state is possible unless you're running it outside of gmod, or manually assigning it

Then if it's unused you could remove it or add a comment saying so and leave it for later

[Vurv78]

Looks like an improvement, but is a huge cost to performance as well. I'd like to see the 'Emit' shit replaced with actual lua rather than string-building into compiled lua code at some point. This is fine for now at least.

Didn't realize it ran at runtime. That is pretty terrible. Then again the emit function itself is very laggy just looking at the code. It uses repeated string concatenation so a string.format call is the least of our problems considering the huge amount of allocations this alone is causing.

I'm fairly certain these will only run on compile/precompile, I.E that section is only encountered for the first time and hasn't been modified by the CPU itself

I've noticed that you can tell easily when something like the GPU is starting a precompile by it flashing the "took too long to draw frame!" the first time you enter a new block.

If it were me I'd refactor it to store it properly as a string buffer and only table.concat when it's needed, and replace the varargs with parameters a-f so the code can jit compile.

If you think this'll help I can do it.

If it's not runtime then it doesn't really matter, so up to you. I just skimmed over the code because @thegrb93 said it'd be a perf hit. I did initially think it only ran to compile once

[DerelictDrone]

I should probably test to check if the emits are run every time, or if they only build a precompiled block, I'm fairly certain it's only while building a precompiled block but it would also be good to see how frequently blocks get precompiled to see how big a hit to performance this may result in.

[thegrb93]

They are precompiled blocks, but it explains the tons of lag you get when initially placing a cpu

[DerelictDrone]

I've got a basic test to measure the amount of time spent in the precompile functions on two separate branches, one before these changes, and one after these changes

Before:

After:

Confirm for yourselves using these two branches perf-testing (before) perf-test-post-emit (after)

[thegrb93]

I wasn't concerned with a huge latency increase, just knew it would be slightly more expensive. I imagine most of the latency is from reallocations.

[thegrb93]

I think vurv just wants a comment added to that emit function and this can be merged.

[DerelictDrone]

Marked them all w/ a comment