search

mt-mods / technic

Technic mod for Minetest
18 stars 25 forks source link

Reduce unnecessary function calls #211

Closed S-S-X closed 2 years ago

S-S-X commented 3 years ago

Simple optimization, allows skipping multiple unnecessary function calls some of which are on expensive side.

BuckarooBanzay commented 3 years ago

Merged this PR on the test-server and got this:

minetest_1             | 2021-09-09 13:28:54: ERROR[Main]: ServerError: AsyncErr: environment_Step: Runtime error from mod 'technic' in callback environment_Step(): ...dmods/technic/technic/machines/register/machine_base.lua:111: attempt to call method 'is_empty' (a nil value)
minetest_1             | 2021-09-09 13:28:54: ERROR[Main]: stack traceback:
minetest_1             | 2021-09-09 13:28:54: ERROR[Main]:      ...dmods/technic/technic/machines/register/machine_base.lua:111: in function <...dmods/technic/technic/machines/register/machine_base.lua:81>
minetest_1             | 2021-09-09 13:28:54: ERROR[Main]:      /data/world//worldmods/technic/technic/machines/network.lua:459: in function 'run_nodes'
minetest_1             | 2021-09-09 13:28:54: ERROR[Main]:      /data/world//worldmods/technic/technic/machines/network.lua:505: in function 'network_run'
minetest_1             | 2021-09-09 13:28:54: ERROR[Main]:      ...echnic/technic/machines/switching_station_globalstep.lua:68: in function 'globalstep'
minetest_1             | 2021-09-09 13:28:54: ERROR[Main]:      /data/world//worldmods/monitoring/builtin/globalstep.lua:32: in function </data/world//worldmods/monitoring/builtin/globalstep.lua:24>
minetest_1             | 2021-09-09 13:28:54: ERROR[Main]:      /usr/local/share/minetest/builtin/game/register.lua:422: in function </usr/local/share/minetest/builtin/game/register.lua:406>
S-S-X commented 3 years ago

Yeah, I've also just noticed that there's no is_empty for inventory lists. However I looked at base machine code bit closer yesterday and decided to go through technic_run function there. Already done some more optimization stuff there and also fixing these invalid inventory list usages.

S-S-X commented 3 years ago

I think I've fixed it... and some additional shortcuts to bypass loops and engine side API access when it is not needed. We'll see if tests pass at least...

S-S-X commented 3 years ago

Btw, this second addition needs testing (processing part). I'm not sure if skipping some core API stuff but using it more in some other situations is better or worse. Did not test that part at all. Actual functionality should be fine but it does affect way how output stacks are filled: always in slot order instead of prioritizing stack with space for item.

If you think it is better then I can split this to original simple bypass for empty machines and another part for actual processing.

S-S-X commented 3 years ago

I think there's something wrong with this, not sure what. It seems to work but have to check and actually do some testing with some pressure for machines based on machine_base to find out real difference.

BuckarooBanzay commented 3 years ago

Updated the technic mod on the test-server with this branch and tested a bit, everything looks good so far, no crashes, malfunctions or duplicated items :+1: Haven't done any useful lag measurements though..

If you think it is better then I can split this to original simple bypass for empty machines and another part for actual processing.

Nah, i think its ok, the diff isn't that huge yet :wink:

S-S-X commented 3 years ago

Updated the technic mod on the test-server with this branch and tested a bit, everything looks good so far, no crashes, malfunctions or duplicated items 👍 Haven't done any useful lag measurements though..

If you think it is better then I can split this to original simple bypass for empty machines and another part for actual processing.

Nah, i think its ok, the diff isn't that huge yet 😉

I'll try to get back to this at some point later, this is only useful if it actually provides better performance for machine execution (there's big potential for this if it does actually limit calling core API functions). However I currently have no idea if those are reduced significantly or not, performance also has to be tested with real engine because it is exactly unnecessary calls to ItemStack and other core stuff.

For ItemStack I've determined before that simple Lua implementation with string parsing (find, sub, match) is at least twice as fast as core API calls. Some functions can be faster when core API is used, it is more about Lua - C++ bridge that slows things down significantly.

github-actions[bot] commented 3 years ago
Click for detailed source code test coverage report ### Test coverage report for Technic CNC 79.48% in 8/8 files: ``` File Hits Missed Coverage ----------------------------------------------------- programs.lua 263 0 100.00% cnc.lua 50 3 94.34% formspec.lua 103 8 92.79% digilines.lua 39 8 82.98% init.lua 19 6 76.00% api.lua 160 83 65.84% pipeworks.lua 25 13 65.79% materials.lua 174 94 64.93% ``` ### Test coverage report for technic 9.60% in 10/103 files: ``` File Hits Missed Coverage -------------------------------------------------------------- machines/HV/generator.lua 9 0 100.00% config.lua 47 4 92.16% machines/register/cables.lua 168 49 77.42% machines/network.lua 192 162 54.24% machines/supply_converter.lua 75 66 53.19% register.lua 20 20 50.00% machines/MV/cables.lua 10 11 47.62% machines/LV/cables.lua 10 11 47.62% machines/HV/cables.lua 9 11 45.00% machines/register/generator.lua 91 114 44.39% util/throttle.lua 0 11 0.00% tools/vacuum.lua 0 32 0.00% tools/tree_tap.lua 0 38 0.00% tools/sonic_screwdriver.lua 0 51 0.00% tools/prospector.lua 0 101 0.00% tools/multimeter.lua 0 208 0.00% tools/mining_lasers.lua 0 65 0.00% tools/mining_drill.lua 0 268 0.00% tools/init.lua 0 14 0.00% tools/flashlight.lua 0 68 0.00% tools/chainsaw.lua 0 115 0.00% tools/cans.lua 0 71 0.00% radiation.lua 0 138 0.00% max_lag.lua 0 12 0.00% machines/switching_station_globalstep.lua 0 58 0.00% machines/switching_station.lua 0 79 0.00% machines/register/solar_array.lua 0 30 0.00% machines/register/recipes.lua 0 78 0.00% machines/register/machine_base.lua 0 171 0.00% machines/register/init.lua 0 22 0.00% machines/register/grindings.lua 0 39 0.00% machines/register/grinder_recipes.lua 0 100 0.00% machines/register/grinder.lua 0 6 0.00% machines/register/freezer_recipes.lua 0 12 0.00% machines/register/freezer.lua 0 6 0.00% machines/register/extractor_recipes.lua 0 71 0.00% machines/register/extractor.lua 0 6 0.00% machines/register/electric_furnace.lua 0 6 0.00% machines/register/compressor_recipes.lua 0 33 0.00% machines/register/compressor.lua 0 6 0.00% machines/register/common.lua 0 114 0.00% machines/register/centrifuge_recipes.lua 0 25 0.00% machines/register/centrifuge.lua 0 6 0.00% machines/register/battery_box.lua 0 238 0.00% machines/register/alloy_recipes.lua 0 40 0.00% machines/register/alloy_furnace.lua 0 30 0.00% machines/power_monitor.lua 0 57 0.00% machines/other/injector.lua 0 85 0.00% machines/other/init.lua 0 8 0.00% machines/other/frames.lua 0 551 0.00% machines/other/constructor.lua 0 103 0.00% machines/other/coal_furnace.lua 0 3 0.00% machines/other/coal_alloy_furnace.lua 0 94 0.00% machines/other/anchor.lua 0 79 0.00% machines/init.lua 0 85 0.00% machines/compat/digtron.lua 0 13 0.00% machines/MV/wind_mill.lua 0 45 0.00% machines/MV/tool_workshop.lua 0 73 0.00% machines/MV/solar_array.lua 0 7 0.00% machines/MV/power_radiator.lua 0 96 0.00% machines/MV/lighting.lua 0 170 0.00% machines/MV/init.lua 0 17 0.00% machines/MV/hydro_turbine.lua 0 44 0.00% machines/MV/grinder.lua 0 6 0.00% machines/MV/generator.lua 0 7 0.00% machines/MV/freezer.lua 0 6 0.00% machines/MV/extractor.lua 0 6 0.00% machines/MV/electric_furnace.lua 0 6 0.00% machines/MV/compressor.lua 0 6 0.00% machines/MV/centrifuge.lua 0 6 0.00% machines/MV/battery_box.lua 0 6 0.00% machines/MV/alloy_furnace.lua 0 6 0.00% machines/LV/water_mill.lua 0 47 0.00% machines/LV/solar_panel.lua 0 27 0.00% machines/LV/solar_array.lua 0 6 0.00% machines/LV/music_player.lua 0 81 0.00% machines/LV/led.lua 0 38 0.00% machines/LV/lamp.lua 0 68 0.00% machines/LV/init.lua 0 17 0.00% machines/LV/grinder.lua 0 7 0.00% machines/LV/geothermal.lua 0 56 0.00% machines/LV/generator.lua 0 7 0.00% machines/LV/extractor.lua 0 13 0.00% machines/LV/electric_furnace.lua 0 6 0.00% machines/LV/compressor.lua 0 10 0.00% machines/LV/battery_box.lua 0 6 0.00% machines/LV/alloy_furnace.lua 0 6 0.00% machines/HV/solar_array.lua 0 6 0.00% machines/HV/quarry.lua 0 306 0.00% machines/HV/nuclear_reactor.lua 0 266 0.00% machines/HV/init.lua 0 12 0.00% machines/HV/grinder.lua 0 6 0.00% machines/HV/forcefield.lua 0 213 0.00% machines/HV/electric_furnace.lua 0 6 0.00% machines/HV/compressor.lua 0 6 0.00% machines/HV/battery_box.lua 0 6 0.00% legacy.lua 0 7 0.00% items.lua 0 51 0.00% integration_test.lua 0 24 0.00% init.lua 0 30 0.00% helpers.lua 0 116 0.00% effects.lua 0 3 0.00% crafts.lua 0 86 0.00% ``` ### Raw test runner output for geeks: CNC: ``` ●●●●●●●●●●●●●●●●●●●●●● 22 successes / 0 failures / 0 errors / 0 pending : 0.207963 seconds ``` Technic: ``` ●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●◌●●●●● 57 successes / 0 failures / 0 errors / 1 pending : 0.168601 seconds Pending → spec/supply_converter_spec.lua @ 99 Supply converter building overloads network spec/supply_converter_spec.lua:99: overload does not work with supply converter ```
S-S-X commented 3 years ago

This is outdated but please do not kill pr/branches just yet, I'd like to keep this as a reminder until affecting pull requests are handled and after that possibly redo things here on top of master branch.