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.

Sign up for GitHub

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

optimize mvps_push_or_pull #662

Closed
wants to merge 11 commits into from
Closed

optimize mvps_push_or_pull #662

Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
00359e7
add profiling debug prints
lolbinarycat Dec 17, 2023
ba9b250
don't check for sticky blocks in locations that are already being pushed
lolbinarycat Dec 17, 2023
bc9ed16
more optimization of inner loops related to mvps_push
lolbinarycat Dec 17, 2023
a7c8146
micro-optimization: eliminate index in inner loop
lolbinarycat Dec 17, 2023
5be0ee9
add fifo_queue.add_list() and use it to eliminate an inner loop
lolbinarycat Dec 17, 2023
43a568d
store result of table lookup in local variable, improving performance…
lolbinarycat Dec 17, 2023
22eae4b
mess with locals and hide debug prints
lolbinarycat Dec 17, 2023
068da0c
cleanup
lolbinarycat Dec 17, 2023
4c27c65
remove profiling stuff
lolbinarycat Dec 17, 2023
79214b5
revert local-variable lifting
lolbinarycat Dec 19, 2023
ab2df3c
add fallback implementation for fifo_queue.add_list() in case table.m…
lolbinarycat Dec 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
cleanup
  • Loading branch information
@lolbinarycat
lolbinarycat committed Dec 17, 2023
commit 068da0c0470efcbc7a2f304292f7633a8d1d7d68
4 changes: 2 additions & 2 deletions mesecons_mvps/init.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@ function mesecon.mvps_get_stack(pos, dir, maximum, all_pull_sticky)
local nodes = {}
local pos_set = {}
local frontiers = mesecon.fifo_queue.new()
--local vector_equals = vector.equals
frontiers:add(vector.new(pos))
-- micro-optimization: lift local definitions out of loop
Copy link
Contributor

@Desour Desour Dec 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be new to me that lifting local variable declarations out of their scope is cheaper. May I ask where you read about this?

(Also, imho it hinders code quality.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is based on personal testing, and is likely not worth it.

unless LuaJIT automatically does this, a local variable would have to have space for it allocated, which necessarily takes time.

local nodedef
local np_hash
local nn
Expand Down Expand Up @@ -100,7 +100,7 @@ function mesecon.mvps_get_stack(pos, dir, maximum, all_pull_sticky)
-- If adjacent node is sticky block and connects add that
-- position
for _, r in ipairs(mesecon.rules.alldirs) do
local adjpos = vector.add(np, r)
adjpos = vector.add(np, r)
-- optimization: don't check blocks that are already part of the stack
if not pos_set[minetest.hash_node_position(adjpos)] then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What’s about r==dir?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i don't fully understand, do you mean i should add an additional optimization to skip this code when r==dir, since that block will already have been added to the queue?

i guess that's a missed optimization then, yeah. do you want me to add it?

adjnode = minetest.get_node(adjpos)
Expand Down