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

Jump to bottom

Rewrite Project #599

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from
Draft

Rewrite Project #599

wants to merge 11 commits into from
+5,800 −2,546

Conversation

rj00a
Copy link
Member

@rj00a rj00a commented Feb 3, 2024 •
edited

Rewrite the project from the ground up. Closes #596

Steps

  • Update data extractor to the latest version and fix known issues.
  • Fix issues in java_string.
  • Rework interface of valence_nbt to support redesign of valence_protocol.
  • Remove valence_generated.
    • Computing everything statically doesn't work in a world where more and more registries are networked.
    • Compile times were too long. We can generate stuff from the JSON at runtime if we need to.
  • Redesign valence_protocol and update to 1.20.4.
    • Reorganize packets into a single module. (Minecraft shares some packets between protocol stages now, so this accommodates for that).
    • Introduce McRead and McWrite traits similar to PacketByteBuf in vanilla.
    • Remove bespoke derive macro for "encodable" and "decodable" types and write all the packet impls out manually.
    • Remove dependence on valence_generated, bevy_ecs, valence_math.
    • Consider merging valence_text and valence_ident into valence_protocol since we don't have valence_generated anymore.
  • Update the packet inspector and get it to a usable state.
    • Create "packet registry" in valence_protocol so the inspector can access all packets programmatically.
  • Rewrite main server crates.
    • Replace bevy_ecs with evenio. Use event driven design described in Rewrite the project with evenio #596.
    • Use monoio + ureq on Linux and MacOS, tokio everywhere else.
    • Handle switching between configuration phase and play phase.
    • Get basic chunk stuff working at least.
    • Combine the miscellaneous crates together like weather, player_list, boss_bar, registry, etc.
  • Pick a linear algebra crate (glam, vek, ultraviolet, something else?)
  • Port some of the examples over.
  • Update valence_anvil (or leave that for follow up?)
  • Update CI scripts.

This was linked to issues Feb 11, 2024
Use new [lints] table in Cargo.toml #586
Open
Make valence_protocol usable for other projects #585
Open
Resolve system order ambiguities #542
Open
Use completion-based IO in valence_network #489
Open
More generic layers #443
Open
Extractor output is non-determinisic. #359
Open
Set up CI for the extractor. #374
Open
Use a Fixed Timestep #327
Open
Digging EventReader has incorrect state when block is instant mined #333
Open
Add missing functionality to tools/stresser #347
Open
@rj00a rj00a removed a link to an issue Feb 11, 2024
Digging EventReader has incorrect state when block is instant mined #333
Open
@rj00a rj00a linked an issue Feb 11, 2024 that may be closed by this pull request
Improve client events #256
Open
1 task
@rj00a rj00a removed a link to an issue Feb 11, 2024
Improve client events #256
Open
1 task
@rj00a rj00a linked an issue Feb 11, 2024 that may be closed by this pull request
Poor performance in bench_players example. #323
Open
@rj00a rj00a mentioned this pull request Mar 9, 2024
Open
@andrewgazelka
Copy link

andrewgazelka commented Mar 17, 2024 •
edited

Given that you are making a bunch of changes and performance is important, have you considered adding benching to CI? I feel like I have seen Rust projects that have good performance regression detection in CI, but I might need to do a bit of searching.

This would be nice because performance would be repeatable (hopefully!) unlike

if I have singleplayer Minecraft open on my machine at the same time I'm running bench_players, the performance actually improves.

#323

@rj00a
Copy link
Member Author

rj00a commented Mar 22, 2024

@andrewgazelka I'm open to the idea, but it seems like a lot of work to do it correctly. For starters we would need dedicated CI runners and a decent suite of benches. Definitely something to think about after the rewrite is finished.

@A-z-z-y
Copy link

A-z-z-y commented Mar 22, 2024

The criterion crate sounds like it may be useful here, unless ya'll have your own favorite (micro)benchmarking tools.

@andrewgazelka
Copy link

@andrewgazelka I'm open to the idea, but it seems like a lot of work to do it correctly. For starters we would need dedicated CI runners and a decent suite of benches. Definitely something to think about after the rewrite is finished.

That makes sense. Why would you need dedicated CI runners, though? Do you think performance would not be predictable using the default CI runners, which run in a VM on the same machine as other runners?

@A-z-z-y
Copy link

A-z-z-y commented Mar 23, 2024

If consistent environments are available (e.g. on your own machine), it may be worth it to use a microbenchmarking crate like criterion to check for performance regressions.

@rj00a
Copy link
Member Author

rj00a commented Mar 25, 2024

Do you think performance would not be predictable using the default CI runners, which run in a VM on the same machine as other runners?

That's right, it's too noisy from what I've heard.

@A-z-z-y I'm quite fond of divan nowadays :)

@splurf
Copy link

splurf commented Apr 3, 2024 •
edited

The crates glam and nalgebra are both well-maintained and highly optimized. There's a benchmark that compares a bunch of popular lin alg crates but it's over 3 years old.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@rj00a @andrewgazelka @A-z-z-y @splurf