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.

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

prost-build: optimise derived prost::Name #956

Merged
merged 3 commits into from
Feb 20, 2024
Merged

Conversation

mina86
Copy link
Contributor

@mina86 mina86 commented Dec 7, 2023

When deriving prost::Name, prost-bulid generates the following code
for full_name and type_url methods:

fn full_name() -> ::prost::alloc::string::String {
    ::prost::alloc::format!(
        "some.package.{0}", Self::NAME
    )
}
fn type_url() -> ::prost::alloc::string::String {
    ::prost::alloc::format!(
        "example.com/{0}", Self::full_name()
    )
}

With those definitions, type_url:

  • allocates a new temporary string,
  • formats text into that string,
  • allocates another string,
  • and once again formats text into that string.

Most of those operations can be done at build time with only a single
string allocation being necessary in type_url method.

Change the generated code such that no formatting happens at run time
and no temporary strings are allocated in type_url, specifically:

fn full_name() -> ::prost::alloc::string::String {
    "some.package.MessageName".into()
}
fn type_url() -> ::prost::alloc::string::String {
    "example.com/some.package.MessageName"
}

Furthermore, unconditionally derive type_url even if no domain is
specified such that default definition (which uses temporary strings
and formatting) isn’t used.

Copy link
Collaborator

@caspermeijn caspermeijn left a comment

Choose a reason for hiding this comment

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

The codebase already has tests for this functionality.

LGTM

prost-build/src/code_generator.rs Outdated Show resolved Hide resolved
When deriving prost::Name, prost-bulid generates the following code
for full_name and type_url methods:

    fn full_name() -> ::prost::alloc::string::String {
        ::prost::alloc::format!(
            "some.package.{0}", Self::NAME
        )
    }
    fn type_url() -> ::prost::alloc::string::String {
        ::prost::alloc::format!(
            "example.com/{0}", Self::full_name()
        )
    }

With those definitions, type_url:
- allocates a new temporary string,
- formats text into that string,
- allocates another string,
- and once again formats text into that string.

Most of those operations can be done at build time with only a single
string allocation being necessary in type_url method.

Change the generated code such that no formatting happens at run time
and no temporary strings are allocated in type_url, specifically:

    fn full_name() -> ::prost::alloc::string::String {
        "some.package.MessageName".into()
    }
    fn type_url() -> ::prost::alloc::string::String {
        "example.com/some.package.MessageName"
    }

Furthermore, unconditionally derive type_url even if no domain is
specified such that default definition (which uses temporary strings
and formatting) isn’t used.
@mina86
Copy link
Contributor Author

mina86 commented Feb 9, 2024

error: package half v2.3.1 cannot be built because it requires rustc 1.70 or newer, while the currently active rustc version is 1.65.0

What about this failure then?

@caspermeijn
Copy link
Collaborator

error: package half v2.3.1 cannot be built because it requires rustc 1.70 or newer, while the currently active rustc version is 1.65.0

What about this failure then?

It seems the MSRV is no longer accurate. This PR is trying to fix that: #975

@caspermeijn
Copy link
Collaborator

Please rebase to the latest master branch to fix CI

Copy link
Collaborator

@caspermeijn caspermeijn left a comment

Choose a reason for hiding this comment

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

This code doesn't change the behavior and already has tests.

@LucioFranco LucioFranco merged commit 0a64a09 into tokio-rs:master Feb 20, 2024
11 checks passed
caspermeijn added a commit to caspermeijn/prost that referenced this pull request Apr 2, 2024
_PROST!_ is a [Protocol Buffers](https://developers.google.com/protocol-buffers/) implementation for the [Rust Language](https://www.rust-lang.org/). `prost` generates simple, idiomatic Rust code from `proto2` and `proto3` files.

This patch updates brings a few new features and fixes:

- Bump MSRV to 1.70 (minimum supported Rust version)
- Rename cargo feature `prost-derive` to `derive` (tokio-rs#992)
- Add @generated comment on top of generated files (tokio-rs#935)
- Optimize implementation of prost::Name when generated by prost-build (tokio-rs#956)

## Dependencies
- build(deps): Allow itertools 0.12 (tokio-rs#948)
- build(deps): Allow heck 0.5 (tokio-rs#1012)
- build(deps): Allow multimap 0.10 (tokio-rs#1013)

## Documentation
- Improve protoc not found error message (tokio-rs#937)
- build: Add development container config (tokio-rs#949)
- docs: Fixed README typos (tokio-rs#952 / tokio-rs#967 / tokio-rs#970)

## Internal
- chore: Fix minimal versions (tokio-rs#920)
- fix: fq_message_name should begin with one dot (tokio-rs#981)
- improve encode_varint performance by bounding its loop (tokio-rs#940)
- style: Remove duplicate function call (tokio-rs#989)
- test: Improve test decode_varint_slow (tokio-rs#977)
- chore: Add dep: prefix to feature dependencies (tokio-rs#919)
- Minor clippy lint fixes. (tokio-rs#1006)
- chore: Use taiki-e/install-action to setup cargo-machete (tokio-rs#909)
- chore: Remove which dependency. (tokio-rs#962)
- chore: Update to actions/checkout@v4 (tokio-rs#910)

```
$ cargo semver-checks --workspace
     Parsing prost v0.12.4 (current)
      Parsed [   4.348s] (current)
     Parsing prost v0.12.3 (baseline, cached)
      Parsed [   0.099s] (baseline)
    Checking prost v0.12.3 -> v0.12.4 (minor change)
     Checked [   0.029s] 61 checks; 61 passed, 6 unnecessary
    Finished [   4.481s] prost
     Parsing prost-build v0.12.4 (current)
      Parsed [   5.488s] (current)
     Parsing prost-build v0.12.3 (baseline)
      Parsed [   3.819s] (baseline)
    Checking prost-build v0.12.3 -> v0.12.4 (minor change)
     Checked [   0.005s] 61 checks; 61 passed, 6 unnecessary
    Finished [   9.319s] prost-build
     Parsing prost-types v0.12.4 (current)
      Parsed [   3.150s] (current)
     Parsing prost-types v0.12.3 (baseline)
      Parsed [   3.192s] (baseline)
    Checking prost-types v0.12.3 -> v0.12.4 (minor change)
     Checked [   0.075s] 61 checks; 61 passed, 6 unnecessary
    Finished [   6.434s] prost-types
```
caspermeijn added a commit to caspermeijn/prost that referenced this pull request Apr 5, 2024
_PROST!_ is a [Protocol Buffers](https://developers.google.com/protocol-buffers/) implementation for the [Rust Language](https://www.rust-lang.org/). `prost` generates simple, idiomatic Rust code from `proto2` and `proto3` files.

This patch update brings new features and fixes:

- Bump MSRV to 1.70 (minimum supported Rust version)
- Rename cargo feature `prost-derive` to `derive` (tokio-rs#992)
- Add @generated comment on top of generated files (tokio-rs#935)
- Optimize implementation of prost::Name when generated by prost-build (tokio-rs#956)

## Dependencies
- build(deps): Allow itertools 0.12 (tokio-rs#948)
- build(deps): Allow heck 0.5 (tokio-rs#1012)
- build(deps): Allow multimap 0.10 (tokio-rs#1013)

## Documentation
- Improve protoc not found error message (tokio-rs#937)
- build: Add development container config (tokio-rs#949)
- docs: Fixed README typos (tokio-rs#952 / tokio-rs#967 / tokio-rs#970)

## Internal
- chore: Fix minimal versions (tokio-rs#920)
- fix: fq_message_name should begin with one dot (tokio-rs#981)
- improve encode_varint performance by bounding its loop (tokio-rs#940)
- style: Remove duplicate function call (tokio-rs#989)
- test: Improve test decode_varint_slow (tokio-rs#977)
- chore: Add dep: prefix to feature dependencies (tokio-rs#919)
- Minor clippy lint fixes. (tokio-rs#1006)
- chore: Use taiki-e/install-action to setup cargo-machete (tokio-rs#909)
- chore: Remove which dependency. (tokio-rs#962)
- chore: Update to actions/checkout@v4 (tokio-rs#910)
caspermeijn added a commit to caspermeijn/prost that referenced this pull request Apr 5, 2024
_PROST!_ is a [Protocol Buffers](https://developers.google.com/protocol-buffers/) implementation for the [Rust Language](https://www.rust-lang.org/). `prost` generates simple, idiomatic Rust code from `proto2` and `proto3` files.

This patch update brings new features and fixes:

- Bump MSRV to 1.70 (minimum supported Rust version)
- Rename cargo feature `prost-derive` to `derive` (tokio-rs#992)
- Add @generated comment on top of generated files (tokio-rs#935)
- Optimize implementation of prost::Name when generated by prost-build (tokio-rs#956)

## Dependencies
- build(deps): Allow itertools 0.12 (tokio-rs#948)
- build(deps): Allow heck 0.5 (tokio-rs#1012)
- build(deps): Allow multimap 0.10 (tokio-rs#1013)

## Documentation
- Improve protoc not found error message (tokio-rs#937)
- build: Add development container config (tokio-rs#949)
- docs: Fixed README typos (tokio-rs#952 / tokio-rs#967 / tokio-rs#970)

## Internal
- chore: Fix minimal versions (tokio-rs#920)
- fix: fq_message_name should begin with one dot (tokio-rs#981)
- improve encode_varint performance by bounding its loop (tokio-rs#940)
- style: Remove duplicate function call (tokio-rs#989)
- test: Improve test decode_varint_slow (tokio-rs#977)
- chore: Add dep: prefix to feature dependencies (tokio-rs#919)
- Minor clippy lint fixes. (tokio-rs#1006)
- chore: Use taiki-e/install-action to setup cargo-machete (tokio-rs#909)
- chore: Remove which dependency. (tokio-rs#962)
- chore: Update to actions/checkout@v4 (tokio-rs#910)
github-merge-queue bot pushed a commit that referenced this pull request Apr 5, 2024
_PROST!_ is a [Protocol Buffers](https://developers.google.com/protocol-buffers/) implementation for the [Rust Language](https://www.rust-lang.org/). `prost` generates simple, idiomatic Rust code from `proto2` and `proto3` files.

This patch update brings new features and fixes:

- Bump MSRV to 1.70 (minimum supported Rust version)
- Rename cargo feature `prost-derive` to `derive` (#992)
- Add @generated comment on top of generated files (#935)
- Optimize implementation of prost::Name when generated by prost-build (#956)

## Dependencies
- build(deps): Allow itertools 0.12 (#948)
- build(deps): Allow heck 0.5 (#1012)
- build(deps): Allow multimap 0.10 (#1013)

## Documentation
- Improve protoc not found error message (#937)
- build: Add development container config (#949)
- docs: Fixed README typos (#952 / #967 / #970)

## Internal
- chore: Fix minimal versions (#920)
- fix: fq_message_name should begin with one dot (#981)
- improve encode_varint performance by bounding its loop (#940)
- style: Remove duplicate function call (#989)
- test: Improve test decode_varint_slow (#977)
- chore: Add dep: prefix to feature dependencies (#919)
- Minor clippy lint fixes. (#1006)
- chore: Use taiki-e/install-action to setup cargo-machete (#909)
- chore: Remove which dependency. (#962)
- chore: Update to actions/checkout@v4 (#910)
gibbz00 pushed a commit to gibbz00/prost that referenced this pull request Apr 6, 2024
_PROST!_ is a [Protocol Buffers](https://developers.google.com/protocol-buffers/) implementation for the [Rust Language](https://www.rust-lang.org/). `prost` generates simple, idiomatic Rust code from `proto2` and `proto3` files.

This patch update brings new features and fixes:

- Bump MSRV to 1.70 (minimum supported Rust version)
- Rename cargo feature `prost-derive` to `derive` (tokio-rs#992)
- Add @generated comment on top of generated files (tokio-rs#935)
- Optimize implementation of prost::Name when generated by prost-build (tokio-rs#956)

## Dependencies
- build(deps): Allow itertools 0.12 (tokio-rs#948)
- build(deps): Allow heck 0.5 (tokio-rs#1012)
- build(deps): Allow multimap 0.10 (tokio-rs#1013)

## Documentation
- Improve protoc not found error message (tokio-rs#937)
- build: Add development container config (tokio-rs#949)
- docs: Fixed README typos (tokio-rs#952 / tokio-rs#967 / tokio-rs#970)

## Internal
- chore: Fix minimal versions (tokio-rs#920)
- fix: fq_message_name should begin with one dot (tokio-rs#981)
- improve encode_varint performance by bounding its loop (tokio-rs#940)
- style: Remove duplicate function call (tokio-rs#989)
- test: Improve test decode_varint_slow (tokio-rs#977)
- chore: Add dep: prefix to feature dependencies (tokio-rs#919)
- Minor clippy lint fixes. (tokio-rs#1006)
- chore: Use taiki-e/install-action to setup cargo-machete (tokio-rs#909)
- chore: Remove which dependency. (tokio-rs#962)
- chore: Update to actions/checkout@v4 (tokio-rs#910)
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.

3 participants