Rebase Nullable #1
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes google#161 --------- Co-authored-by: Ömer Sinan Ağacan <omersa@google.com>
…ields (google#863) Currently unknown field set parser stores length-delimited fields as views of the input buffer. This has a few issues: - It's inconsistent with the known-field parsing where we copy `bytes` fields. - A single unknown length-delimited field can cause keeping a large input buffer alive. - Because the caller is free to modify the input buffer, this implementation does not allow freezing an unknown field set. (This can also be fixed by copying the length-delimited fields when freezing.) - Even when the parsed message is not frozen, this aliasing can cause bugs as it's not documented. Even if we document it, it would probably be a footgun. This can cause segfaults or worse when the input buffer is passed from e.g. C++ or C as a `Uint8List`, and the caller frees the buffer after parsing while the message is still in use. This PR makes `readBytes` copy the bytes before returning to avoid aliasing. A new member `readBytesAsView` added with the previous behavior. Unknown length-delimited field parsing fixed with the new `readBytes`. Sync CL: cl/552077275
See changelog for details. Fixes google#850.
…ate protos (google#869) - Rename a local variable to avoid proto field name conflicts. This fixes an issue we're seeing with the latest version of `package:protoc_plugin` and the dartpad proto definitions. - Re-generate descriptor protos. - Update CI config to re-run publishing when labels change.
…e#870) We used to add a prefix before, but when I brought back the constructor arguments I didn't include it because (1) the previous prefix was `_` which caused lint errors (2) there were no tests or documentation on why the prefix was needed. Document it now so that it won't be removed again. (I couldn't add this in google#869 as I didn't have push access to the PR's repo)
This updates benchmark builder with two dart2wasm targets: - wasm: Default optimized build. - wasm-omit-checks: Optimized build with `--omit-checks`. README is updated with instructions to build and run new targets.
Bumps [actions/checkout](https://github.com/actions/checkout) from 3.5.2 to 3.6.0. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/actions/checkout/releases">actions/checkout's releases</a>.</em></p> <blockquote> <h2>v3.6.0</h2> <h2>What's Changed</h2> <ul> <li>Mark test scripts with Bash'isms to be run via Bash by <a href="https://github.com/dscho"><code>@dscho</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/1377">actions/checkout#1377</a></li> <li>Add option to fetch tags even if fetch-depth > 0 by <a href="https://github.com/RobertWieczoreck"><code>@RobertWieczoreck</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/579">actions/checkout#579</a></li> <li>Release 3.6.0 by <a href="https://github.com/luketomlinson"><code>@luketomlinson</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/1437">actions/checkout#1437</a></li> </ul> <h2>New Contributors</h2> <ul> <li><a href="https://github.com/RobertWieczoreck"><code>@RobertWieczoreck</code></a> made their first contribution in <a href="https://redirect.github.com/actions/checkout/pull/579">actions/checkout#579</a></li> <li><a href="https://github.com/luketomlinson"><code>@luketomlinson</code></a> made their first contribution in <a href="https://redirect.github.com/actions/checkout/pull/1437">actions/checkout#1437</a></li> </ul> <p><strong>Full Changelog</strong>: <a href="https://github.com/actions/checkout/compare/v3.5.3...v3.6.0">https://github.com/actions/checkout/compare/v3.5.3...v3.6.0</a></p> <h2>v3.5.3</h2> <h2>What's Changed</h2> <ul> <li>Fix: Checkout Issue in self hosted runner due to faulty submodule check-ins by <a href="https://github.com/megamanics"><code>@megamanics</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/1196">actions/checkout#1196</a></li> <li>Fix typos found by codespell by <a href="https://github.com/DimitriPapadopoulos"><code>@DimitriPapadopoulos</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/1287">actions/checkout#1287</a></li> <li>Add support for sparse checkouts by <a href="https://github.com/dscho"><code>@dscho</code></a> and <a href="https://github.com/dfdez"><code>@dfdez</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/1369">actions/checkout#1369</a></li> <li>Release v3.5.3 by <a href="https://github.com/TingluoHuang"><code>@TingluoHuang</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/1376">actions/checkout#1376</a></li> </ul> <h2>New Contributors</h2> <ul> <li><a href="https://github.com/megamanics"><code>@megamanics</code></a> made their first contribution in <a href="https://redirect.github.com/actions/checkout/pull/1196">actions/checkout#1196</a></li> <li><a href="https://github.com/DimitriPapadopoulos"><code>@DimitriPapadopoulos</code></a> made their first contribution in <a href="https://redirect.github.com/actions/checkout/pull/1287">actions/checkout#1287</a></li> <li><a href="https://github.com/dfdez"><code>@dfdez</code></a> made their first contribution in <a href="https://redirect.github.com/actions/checkout/pull/1369">actions/checkout#1369</a></li> </ul> <p><strong>Full Changelog</strong>: <a href="https://github.com/actions/checkout/compare/v3...v3.5.3">https://github.com/actions/checkout/compare/v3...v3.5.3</a></p> </blockquote> </details> <details> <summary>Changelog</summary> <p><em>Sourced from <a href="https://github.com/actions/checkout/blob/main/CHANGELOG.md">actions/checkout's changelog</a>.</em></p> <blockquote> <h1>Changelog</h1> <h2>v3.6.0</h2> <ul> <li><a href="https://redirect.github.com/actions/checkout/pull/1377">Fix: Mark test scripts with Bash'isms to be run via Bash</a></li> <li><a href="https://redirect.github.com/actions/checkout/pull/579">Add option to fetch tags even if fetch-depth > 0</a></li> </ul> <h2>v3.5.3</h2> <ul> <li><a href="https://redirect.github.com/actions/checkout/pull/1196">Fix: Checkout fail in self-hosted runners when faulty submodule are checked-in</a></li> <li><a href="https://redirect.github.com/actions/checkout/pull/1287">Fix typos found by codespell</a></li> <li><a href="https://redirect.github.com/actions/checkout/pull/1369">Add support for sparse checkouts</a></li> </ul> <h2>v3.5.2</h2> <ul> <li><a href="https://redirect.github.com/actions/checkout/pull/1289">Fix api endpoint for GHES</a></li> </ul> <h2>v3.5.1</h2> <ul> <li><a href="https://redirect.github.com/actions/checkout/pull/1246">Fix slow checkout on Windows</a></li> </ul> <h2>v3.5.0</h2> <ul> <li><a href="https://redirect.github.com/actions/checkout/pull/1237">Add new public key for known_hosts</a></li> </ul> <h2>v3.4.0</h2> <ul> <li><a href="https://redirect.github.com/actions/checkout/pull/1209">Upgrade codeql actions to v2</a></li> <li><a href="https://redirect.github.com/actions/checkout/pull/1210">Upgrade dependencies</a></li> <li><a href="https://redirect.github.com/actions/checkout/pull/1225">Upgrade <code>@actions/io</code></a></li> </ul> <h2>v3.3.0</h2> <ul> <li><a href="https://redirect.github.com/actions/checkout/pull/1045">Implement branch list using callbacks from exec function</a></li> <li><a href="https://redirect.github.com/actions/checkout/pull/1050">Add in explicit reference to private checkout options</a></li> <li>[Fix comment typos (that got added in <a href="https://redirect.github.com/actions/checkout/issues/770">#770</a>)](<a href="https://redirect.github.com/actions/checkout/pull/1057">actions/checkout#1057</a>)</li> </ul> <h2>v3.2.0</h2> <ul> <li><a href="https://redirect.github.com/actions/checkout/pull/942">Add GitHub Action to perform release</a></li> <li><a href="https://redirect.github.com/actions/checkout/pull/967">Fix status badge</a></li> <li><a href="https://redirect.github.com/actions/checkout/pull/1002">Replace datadog/squid with ubuntu/squid Docker image</a></li> <li><a href="https://redirect.github.com/actions/checkout/pull/964">Wrap pipeline commands for submoduleForeach in quotes</a></li> <li><a href="https://redirect.github.com/actions/checkout/pull/1029">Update <code>@actions/io</code> to 1.1.2</a></li> <li><a href="https://redirect.github.com/actions/checkout/pull/1039">Upgrading version to 3.2.0</a></li> </ul> <h2>v3.1.0</h2> <ul> <li><a href="https://redirect.github.com/actions/checkout/pull/939">Use <code>@actions/core</code> <code>saveState</code> and <code>getState</code></a></li> <li><a href="https://redirect.github.com/actions/checkout/pull/922">Add <code>github-server-url</code> input</a></li> </ul> <h2>v3.0.2</h2> <ul> <li><a href="https://redirect.github.com/actions/checkout/pull/770">Add input <code>set-safe-directory</code></a></li> </ul> <h2>v3.0.1</h2> <ul> <li><a href="https://redirect.github.com/actions/checkout/pull/762">Fixed an issue where checkout failed to run in container jobs due to the new git setting <code>safe.directory</code></a></li> <li><a href="https://redirect.github.com/actions/checkout/pull/744">Bumped various npm package versions</a></li> </ul> <h2>v3.0.0</h2> <!-- raw HTML omitted --> </blockquote> <p>... (truncated)</p> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/actions/checkout/commit/f43a0e5ff2bd294095638e18286ca9a3d1956744"><code>f43a0e5</code></a> Release 3.6.0 (<a href="https://redirect.github.com/actions/checkout/issues/1437">#1437</a>)</li> <li><a href="https://github.com/actions/checkout/commit/7739b9ba2efcda9dde65ad1e3c2dbe65b41dfba7"><code>7739b9b</code></a> Add option to fetch tags even if fetch-depth > 0 (<a href="https://redirect.github.com/actions/checkout/issues/579">#579</a>)</li> <li><a href="https://github.com/actions/checkout/commit/96f53100ba2a5449eb71d2e6604bbcd94b9449b5"><code>96f5310</code></a> Mark test scripts with Bash'isms to be run via Bash (<a href="https://redirect.github.com/actions/checkout/issues/1377">#1377</a>)</li> <li><a href="https://github.com/actions/checkout/commit/c85c95e3d7251135ab7dc9ce3241c5835cc595a9"><code>c85c95e</code></a> Release v3.5.3 (<a href="https://redirect.github.com/actions/checkout/issues/1376">#1376</a>)</li> <li><a href="https://github.com/actions/checkout/commit/d106d4669b3bfcb17f11f83f98e1cab478e9f635"><code>d106d46</code></a> Add support for sparse checkouts (<a href="https://redirect.github.com/actions/checkout/issues/1369">#1369</a>)</li> <li><a href="https://github.com/actions/checkout/commit/f095bcc56b7c2baf48f3ac70d6d6782f4f553222"><code>f095bcc</code></a> Fix typos found by codespell (<a href="https://redirect.github.com/actions/checkout/issues/1287">#1287</a>)</li> <li><a href="https://github.com/actions/checkout/commit/47fbe2df0ad0e27efb67a70beac3555f192b062f"><code>47fbe2d</code></a> Fix: Checkout fail in self-hosted runners when faulty submodule are checked-i...</li> <li>See full diff in <a href="https://github.com/actions/checkout/compare/8e5e7e5ab8b370d6c329ec480221332ada57f0ab...f43a0e5ff2bd294095638e18286ca9a3d1956744">compare view</a></li> </ul> </details> <br /> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Event support is currently unused internally and we don't want to introduce new uses of it, to keep the API surface small and the library easier to update, and to make it easier to migrate to another library. `createRepeatedField` and `createMapField` existed to support events (as `PbList` and `PbMap` don't support this), so with the event mixin removed we remove these members as well. Removing these members give us other opportunities: we now have full control over the field value types. This allows, for example, refactoring `PbMap` and `PbList` types for marking them as frozen without visiting the elements, which makes it possible to implement decoders that create frozen objects without having to make another pass after decoding to mark every object as frozen. Closes google#738. cl/571893384
dart2wasm currently can't optimize loops into memcpy, however `setRange` methods have type tests to generate `array.copy` (Wasm's `memcpy`). Replacing the loops in `CodedBufferWriter._copyInto` with `setRange` improves an internal benchmark extracted from a real use case significantly in all targets: | | Before | After | Diff | |------------------------------|------------|------------|---------------------| | AOT | 127,587 us | 95,634 us | -31,953 us, -25.0% | | JIT | 106,880 us | 92,800 us | -14,080 us, -13.1% | | dart2js -O4 | 285,587 us | 262,222 us | -23,365 us, -8.1% | | dart2wasm --omit-type-checks | 337,000 us | 236,100 us | -100,900 us, -29.9% |
…edBufferWriter` (google#887) Similar to google#885, this optimizes some more buffer copying to `memcpy`. Results from the same benchmark in google#885: | | Before | After | Diff | |------------------------------|------------|------------|---------------------| | AOT | 114,713 us | 109,838 us | - 4,875 us, -4.2% | | JIT | 91,960 us | 92,887 us | + 927 us, +1.0% | | dart2js -O4 | 259,125 us | 257,000 us | - 2,125 us, -0.8% | | dart2wasm --omit-type-checks | 196,909 us | 182,333 us | -14,576 us, -7.4% | AOT and JIT tested on x64.
…fferReader` (google#888) Benchmark results from the same internal benchmark reported in previous PRs/commits: | | Before | After | Diff | |------------------------------|------------|------------|---------------------| | AOT | 31,025 us | 27,597 us | - 3,428 us, -11.0% | | JIT | 34,829 us | 26,846 us | - 7,983 us, -22.9% | | dart2js -O4 | 300,571 us | 248,300 us | -52,271 us, -17.3% | | dart2wasm --omit-type-checks | 130,812 us | 107,850 us | -22,962 us, -17.5% |
Ideally `Int64.fromBytes` would provide a `skip` argument (similar to `Utf8Decoder.convert`) to avoid allocating intermediate views completely, but for now we just remove the intermediate `ByteData`.
Leftover change from google#880.
…ogle#891) `writeRawBytes` adds bytes to the output and it will only be fast when the argument is `Uint8List`. Since all the users already pass `Uint8List`, update the argument type. This allows simplifyping `_copyInto`. Because it has only one call site, we simplify it and inline it in the call site. cl/576427817
By avoiding allocating empty `Uint8List`s this saves a few percent in dart2js -O4 in the benchmark reported in google#885. | | Before | After | Diff | |------------------------------|------------|------------|---------------------| | AOT | 122,917 us | 122,741 us | | | JIT | 93,376 us | 94,880 us | | | dart2js -O4 | 271,111 us | 258,250 us | -12,861 us, -4.7% | | dart2wasm --omit-type-checks | 196,454 us | 195,300 us | | Number of splices in the benchmark before this change: 21,123 After: 20,857
…teData` allocs (google#890) In Dart, the only way to convert a list of bytes to an `int` or `double` is by using a `ByteData`. Currently `CodedBufferReader` allocates a new `ByteData` for every fixed-size `int` or `float` decoding. With this change we now allocate a `ByteData` once initialization and reuse it. Benchmark results, using the same benchmark reported in google#888: | | Before | After | Diff | |------------------------------|------------|------------|---------------------| | AOT | 26,763 us | 25,305 us | - 1,458 us, - 5.4% | | JIT | 24,573 us | 25,521 us | + 948 us, + 3.8% | | dart2js -O4 | 212,000 us | 199,300 us | -12,700 us, - 5.9% | | dart2wasm --omit-type-checks | 89,750 us | 78,958 us | -10,792 us, -12.0% |
Since groups are deprecated I don't have any benchmarks for this change. However it should actually be faster on JS and Wasm as `is UnknownFieldSet` is just one class id comparison, and the `else` branch won't have a type check in dart2js and dart2wasm.
In `PbList`, the list field becomes monomorphic growable list (from the list base class). This makes `add` calls monomorphic and inlinable, and avoids double mutability checks (once in `PbList.add`, again in `_wrappedList.add`). Also simplifies immutable `PbMap` allocations. `PbMap._isReadonly` is renamed to `_isReadOnly` for consistency with `PbList._isReadOnly`.
This improves iteration performance by making `moveNext` and `current` direct calls in kernel. google#902 does not have the same effect on `moveNext` and `current` calls, this change is needed even with google#902. This change was not possible before google#880 as users could override the list and map types to types that are not `PbList`s or `PbMap`s. Note: changes in the message field types (the plugin changes in this PR) are not necessary for the kernel improvements, but it doesn't hurt to generate more precise types everywhere.
Changing `Map.unmodifiable` to a static `<Never, Never>{}` causes runtime type
errors in `firstWhere` uses, in code like `map.keys.firstWhere(f1, orElse: f2)`.
---
cl/585873554
- Reduce binary size increase caused by cl/586928869 by adding `dart2js:never-inline` pragmas to `add`, `addAll`, `clear`. - Override `isEmpty`, `isNotEmpty` avoid virtual calls to `length`. - Override `get iterator` to use the known iterator type of `_wrappedList`, which is always a `_GrowableList`. cl/587627017
…tion to hazzers. Signed-off-by: Johyn Papin <contact@johyn.me>
Signed-off-by: Johyn Papin <contact@johyn.me>
Signed-off-by: Johyn Papin <contact@johyn.me>
note that right now, this test is failing
Currently when a user calls deprecated methods `clone` or `copyWith`,
the deprecation message points to `GeneratedMessageGenericExtensions`
`deepCopy` and `rebuild` methods.
However we can't just replace `clone` with `deepCopy` as currently the
extension is not exported by the generated files. Instead we need to
import the library explicitly in the use site.
We could mention this in the deprecation message ("Use rebuild from
protobuf library instead"), but it's more convenient to just export the
extension in the generated message files.
Closes google#503.
This implements generating doc comments for enum types, enum values, rpc types, and rpc methods. Existing code for generating doc comments is refactored: the comment generator handles trailing whitespace so we don't have to handle it in the call sites. Comment generator is documented with the invariants: it never returns an empty string (returns `null` instead) and never returns lines with trailing whitespace. To be able to test comment generating, a new kind of golden file test is added. The difference from the other golden file tests is that this file is compiled from .proto source instead of from a hand-crafted descriptor. This is because attaching comments to proto field and types by hand is difficult and error prone. Fixes the comment part of google#900.
…d values, messages (google#908) Similar to google#909, a new kind of golden test is added here where the test input is not a hand-crafted message descriptor but an actual .proto file.
ghost
merged commit
This pull request was closed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Rebase on master