PaperMC / PaperMC/Paper

Default to not creating snapshots in BlockState getters

Open
#11,912 3 comments 3 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

status: input wanted
Dominant language
Java
Stars
12.7k
Forks
3.5k
Avg merge
3d 13h
Merged PRs (30d)
11

Description

Is your feature request related to a problem?

Around a decade ago the Bukkit API swapped to creating snapshots when creating BlockState objects with a raw getState() method, which was magnitudes slower than beforehand. Paper created a getState(boolean) method as a workaround, to allow plugins to opt-out of the snapshot behaviour. This is good, but many plugins don't realise and instead opt for the significantly slower snapshot variant.

Describe the solution you'd like.

Paper should change the default behaviour to not create snapshots, and instead have usecases that require snapshots go through getState(true) instead. This would make code performant by default, which fits Paper's general goals.

This change should be made across all method calls that create a BlockState instance, not just Block#getState().

Describe alternatives you've considered.

An alternative would be to deprecate the raw method entirely, so that developers are forced to confront the choice of "do i need a snapshot or not". This of course would mean that the change would not affect existing plugins, and would require developers to fix things.

Other

This should ideally not be too breaking of a change. A vast majority of plugins function identically with or without snapshots.

When the snapshot system was initially created, a Sign was 1600x slower than the non-snapshot equivalent. Signs being one of the simplest tile entities at the time. The game has only increased in complexity, and the slowdown is entirely dependent on the types and quantity of data that are being pulled in. In 1.21.4 chests can be upwards of 100000 times slower depending on what's inside them. I've seen numerous Spark reports that show half the entire server tick being used entirely by plugins doing a handful of chest accesses on chests that contain a large amount of data.

I've created a branch that appears to do this, but i haven't fully gone through and checked that it covers absolutely every code path. PR at https://github.com/PaperMC/Paper/pull/11913; should have a PR jar attached if someone wants to test it out with their plugins/server setups.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reviewing the existing BlockState getter behavior and the proposed changes in PR #11913. Check whether every method call that creates a BlockState is covered, then test the PR jar with representative plugins and server setups to confirm snapshot-dependent use cases still work and default getters avoid snapshots.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
api, performance
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.