Skip to content

Don't use streams for VoxelShape calls#2355

Closed
Cryptite wants to merge 6 commits into
PaperMC:masterfrom
Cryptite:nostreams
Closed

Don't use streams for VoxelShape calls#2355
Cryptite wants to merge 6 commits into
PaperMC:masterfrom
Cryptite:nostreams

Conversation

@Cryptite
Copy link
Copy Markdown
Contributor

@Cryptite Cryptite commented Jul 22, 2019

Per @electronicboy's suggestion.

Streams aren't performant with hot code and Mojang uses them quite frequently. It was suggested to try to revert stream calls in VoxelShape back to traditional for loops and Lists. Doing so seems to have increased the speed of Entity.move() from 13-22% on some cursory testing:

With 1.13.2 latest Paper (about 200.8ms per call when sampling)
image

With this patch (about 175.1ms per call when sampling)
image

The testing scenario for these profilings were just a simply small pen of about 195 or so Pigs:
image

Comment thread Spigot-Server-Patches/0445-Don-t-use-streams-for-VoxelShape-calls.patch Outdated
Comment thread scripts/importmcdev.sh Outdated
Comment thread Spigot-Server-Patches/0445-Don-t-use-streams-for-VoxelShape-calls.patch Outdated
@Spottedleaf
Copy link
Copy Markdown
Member

Spottedleaf commented Jul 22, 2019

try to keep the ABI intact (i.e convert stream params to list for param forwarding)
for return type changes do the same but have the new method keep a different name

and //paper comments need to be added

@ninja-
Copy link
Copy Markdown

ninja- commented Jul 22, 2019

the last thing mojang cares about is performance. nice find 👍

@Foorcee
Copy link
Copy Markdown
Contributor

Foorcee commented Aug 1, 2019

Is it perhaps also a problem in the 1.14?

@ninja-
Copy link
Copy Markdown

ninja- commented Aug 1, 2019

Is it perhaps also a problem in the 1.14?

bet

@zachbr
Copy link
Copy Markdown
Contributor

zachbr commented Aug 10, 2019

Still lots of broken ABI in this PR

@Cryptite
Copy link
Copy Markdown
Contributor Author

I'm unlikely to revisit this patch for 1.13.2. Welcome to close it or let somebody else take over but I'd rather redo the work in 1.14 or such rather than be behind.

@Spottedleaf
Copy link
Copy Markdown
Member

I've found that this patch is not as relevant in 1.14

@Cryptite Cryptite closed this Aug 10, 2019
@zachbr zachbr self-assigned this Aug 10, 2019
zachbr pushed a commit that referenced this pull request Aug 10, 2019
@Hyronymos-Old
Copy link
Copy Markdown

Hyronymos-Old commented Aug 13, 2019

https://sparkprofiler.github.io/#plFv0FVL4R

It looks like for me that voxelshape is cusing laggs in 1.14.4, isnt it?

MisterVector pushed a commit to MisterVector/Paper that referenced this pull request Dec 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants