Repository navigation
feat!: Add relic adapter based zig std.http and zio - #390
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
addc698 to
8e30e71
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #390 +/- ##
==========================================
- Coverage 92.50% 92.15% -0.36%
==========================================
Files 117 126 +9
Lines 5260 6093 +833
Branches 2634 3037 +403
==========================================
+ Hits 4866 5615 +749
- Misses 394 478 +84
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
8e30e71 to
64bb1fd
Compare
|
Some comparative performance numbers from my trusty old 2,4 GHz 8-Core Intel Core i9 macbook pro (2019).
|
14c4386 to
b9aefcb
Compare
- prefer_final_parameters is off, trailing_commas is automate - doc/site/versioned_docs is excluded
b9aefcb to
f99a6e7
Compare
- dart fix adds const, braces, @OverRide and sorted directives in relic_core, relic_io and benchmark - The examples ignore avoid_print, and packages/examples declares http and web_socket
f99a6e7 to
1b5a584
Compare
- HeaderName with the interned standard names, generated by tool/generate_header_names.dart from tool/header_names.txt - HeaderStore with names and values abstract, MutableHeaderStore with set, add, remove and clear, and MapHeaderStore as the reference store - ByteHeaderSource for a store that keeps the wire bytes of its fields
- Adapter.start takes an ExchangeSink. AdapterExchange carries respond, hijack, abort, cancelled and done, and the core guards every call - The adapter conformance suite moves into test_utils and runs on relic_io, and relic_server_ssl_test.dart holds the TLS tests BREAKING CHANGE: Adapter.requests and AdapterRequest are gone. An adapter implements start(ExchangeSink) and hands over AdapterExchange objects.
- Headers reads through accessors over a HeaderStore, ByteHeaderStore decodes an indexed head as a ByteHeaderSource, and int headers decode from the wire bytes - Headers and MutableHeaders share one read path, Headers.entries is a deprecated extension, and an empty typed value clears the header - RequestTarget keeps the path and query as bytes, and Body.fromBytes keeps its bytes BREAKING CHANGE: Headers is no longer a Map. Read it with accessors or operator [], and write it through Headers.build or transform.
- WebSocketFrameDecoder, WebSocketMessageAssembler and FramedWebSocket implement RFC 6455 over a StreamChannel of bytes, with a 16 MiB message limit, pings, a bounded flush and a bounded close handshake - A missed pong closes the socket at once, and a frame over the message limit is refused before its payload
- Hijack gives the handler a StreamChannel<Uint8List> - A WebSocketUpgrade on an adapter without its own upgrade runs the handshake and a FramedWebSocket over that channel - RelicServer checks the handshake before any adapter upgrade runs and hands the accept key to AdapterExchange.upgradeWebSocket - ExchangeEndState records how an exchange ended and backs done - Request.cancelled completes when the peer goes away
- generate_header_names.dart writes relic_native/src/header_names.zig - The Zig table holds the ids, names and a case-folded lookup
- HeaderSlot carries name_id, from header_names.zig or -1, in place of name_len - parseHead switches on the id for the framing fields, answers 400 to Transfer-Encoding beside Content-Length, and reads Connection as a token list - ByteHeaderStore matches a known name by id through HeaderName.known and interns an unknown one up to the colon - Remove HeaderName.lookupBytes and _eqBytes from header_name.dart
1b5a584 to
631e340
Compare
|
Commits with important interface additions / changes
|
SandPod
left a comment
There was a problem hiding this comment.
Interactive review using Claude Code to inspect the code.
Focused mostly on the interface and behavioral changes.
Trust that our test coverage handles the implementation details.
- IOAdapter.close(force: true) completes a graceful close still waiting - NativeAdapter.close stops the reactor before it waits for a drain turn - The conformance suite covers a forced close during a graceful one and a hijacked peer that never drains BREAKING CHANGE: RelicServer.close() waits with no deadline. Bound it with close().timeout(grace, onTimeout: () => close(force: true)).
- Body.of takes a ContentTypeHeader and bytes or a stream - Headers.contentType is a public accessor - IOAdapter and NativeAdapter read the request Content-Type with tryGet BREAKING CHANGE: Body.ofRequest(headers, ...) is Body.of(headers.tryGet(Headers.contentType), ...).
- _createFileBody takes no isHeadRequest
- The Unreleased section of the root, relic_core, relic_io and relic CHANGELOGs
|
Despite all the breaking changes, both serverpod 4.0.4 and 4.1.0-beta.3 works with this branch. Tested with
The info's are down to the deprecation of Tested on macos and linux (via docker) - not windows. |
|
@coderabbitai Create an issue to upstream zio changes |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
rg -n -C 2 'zio|foreign thread' --glob '!*.lock' --glob '!**/vendor/**' --glob '!**/vendored/**' . | head -240
gh api repos/serverpod/relic/issues/390/comments --paginate --jq '.[] | select(.user.login == "nielsenko") | {html_url, body}'Length of output: 22443 🏁 Script executed: #!/bin/bash
set -euo pipefail
gh issue create \
--repo serverpod/relic \
--title "Upstream the zio changes used by relic_native" \
--assignee nielsenko \
--body "$(cat <<'EOF'
## Summary
Upstream the zio changes introduced for `relic_native` in PR #390, then migrate Relic to an upstream zio revision that includes the required support.
The adapter currently uses a modified zio version so a foreign thread can drive the event loop. This lets the Dart isolate drive its reactor without an extra thread handoff across FFI. These changes have not yet been upstreamed, as described in the PR.
## Required work
- Inventory the local zio changes against the upstream base and separate the reusable changes from Relic-specific integration.
- Propose and submit the required changes to [zio/zio](https://github.com/zio/zio). Document the externally driven event-loop API, ownership rules, and thread-safety constraints.
- Address thread migration: Dart can move an isolate between VM threads. The native build documents that an io_uring ring configured with `SINGLE_ISSUER` cannot be used this way. Include the relevant configuration support in the upstream work, if it is not already available.
- Add upstream tests or examples for externally driven execution, wakeups, shutdown, and sequential driving from different OS threads where supported.
- Once the changes are available upstream, update Relic's zio dependency and adapt the integration to the accepted upstream API.
## Affected areas
- `packages/relic_native/src/relic_native.zig`: per-isolate reactors and the Dart-driven zio runtime integration.
- `packages/relic_native/build.zig`: zio dependency configuration, including the io_uring thread constraint.
- The zio dependency declaration and pinned revision for `relic_native`.
## Acceptance criteria
- [ ] The required zio changes are inventoried and linked to upstream submissions.
- [ ] Upstream includes the support required by Relic, with documented execution and threading contracts.
- [ ] Relic uses an upstream revision without requiring the current private modifications.
- [ ] Native HTTP and WebSocket integration tests pass on macOS and Linux.
- [ ] Windows support is verified separately, or its limitations are explicitly documented.
## Context
- Source PR: https://github.com/serverpod/relic/pull/390
- Request: https://github.com/serverpod/relic/pull/390#issuecomment-6290596864
- Requested by: @nielsenko
EOF
)"Length of output: 2638
The issue covers upstream submissions, threading constraints, tests, and migration to an upstream zio revision. You are interacting with an AI system. |
SandPod
left a comment
There was a problem hiding this comment.
Given the last comment is added, Relic is ready for LIGHTSPEED 🚀
- Add a conformance test that a hijacked channel starts with the unread body of a request with a Content-Length
Adds
relic_native(an HTTP adapter backed by zio and bits from zig'sstd.http), andrelic_headers(a headers abstraction that is more amenable for native inter-op than<String, List<String>>{}An adapter now hands over
AdapterExchangeobjects,Headersreads through aHeaderStore, and the core frames responses and WebSockets for any adapter.The PR uses a tweaked version of zio that allows a foreign thread to drive the event loop (to avoid a thread jump over FFI). These changes have not been upstreamed yet.
Fixes: #316
Breaking:
Adapter.requestsandAdapterRequestare gone, andHeadersis no longer aMap.