[ 
https://issues.apache.org/jira/browse/GROOVY-12033?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18083145#comment-18083145
 ] 

ASF GitHub Bot commented on GROOVY-12033:
-----------------------------------------

codecov-commenter commented on PR #2555:
URL: https://github.com/apache/groovy/pull/2555#issuecomment-4528282986

   ## 
[Codecov](https://app.codecov.io/gh/apache/groovy/pull/2555?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
 Report
   :x: Patch coverage is `82.75862%` with `20 lines` in your changes missing 
coverage. Please review.
   :white_check_mark: Project coverage is 68.1989%. Comparing base 
([`5318145`](https://app.codecov.io/gh/apache/groovy/commit/53181454e08716af1fb76927215775fb75084f56?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache))
 to head 
([`ac1d789`](https://app.codecov.io/gh/apache/groovy/commit/ac1d7893d6d2080d0029e31f94154ff16ff2ff43?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   :warning: Report is 12 commits behind head on master.
   
   | [Files with missing 
lines](https://app.codecov.io/gh/apache/groovy/pull/2555?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
 | Patch % | Lines |
   |---|---|---|
   | 
[.../org/apache/groovy/runtime/async/DefaultActor.java](https://app.codecov.io/gh/apache/groovy/pull/2555?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fapache%2Fgroovy%2Fruntime%2Fasync%2FDefaultActor.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dyb292eS9ydW50aW1lL2FzeW5jL0RlZmF1bHRBY3Rvci5qYXZh)
 | 83.5294% | [12 Missing and 2 partials :warning: 
](https://app.codecov.io/gh/apache/groovy/pull/2555?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
 |
   | 
[src/main/java/groovy/concurrent/Actor.java](https://app.codecov.io/gh/apache/groovy/pull/2555?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Fgroovy%2Fconcurrent%2FActor.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9ncm9vdnkvY29uY3VycmVudC9BY3Rvci5qYXZh)
 | 57.1429% | [6 Missing :warning: 
](https://app.codecov.io/gh/apache/groovy/pull/2555?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
 |
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   [![Impacted file tree 
graph](https://app.codecov.io/gh/apache/groovy/pull/2555/graphs/tree.svg?width=650&height=150&src=pr&token=1r45138NfQ&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)](https://app.codecov.io/gh/apache/groovy/pull/2555?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
   
   ```diff
   @@                Coverage Diff                 @@
   ##               master      #2555        +/-   ##
   ==================================================
   + Coverage     68.1844%   68.1989%   +0.0145%     
   - Complexity      33086      33128        +42     
   ==================================================
     Files            1508       1509         +1     
     Lines          126130     126244       +114     
     Branches        22878      22899        +21     
   ==================================================
   + Hits            86001      86097        +96     
   - Misses          32490      32506        +16     
   - Partials         7639       7641         +2     
   ```
   
   | [Files with missing 
lines](https://app.codecov.io/gh/apache/groovy/pull/2555?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
 | Coverage Δ | |
   |---|---|---|
   | 
[src/main/java/groovy/concurrent/ActorOptions.java](https://app.codecov.io/gh/apache/groovy/pull/2555?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Fgroovy%2Fconcurrent%2FActorOptions.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9ncm9vdnkvY29uY3VycmVudC9BY3Rvck9wdGlvbnMuamF2YQ==)
 | `100.0000% <100.0000%> (ø)` | |
   | 
[src/main/java/groovy/concurrent/Actor.java](https://app.codecov.io/gh/apache/groovy/pull/2555?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Fgroovy%2Fconcurrent%2FActor.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9ncm9vdnkvY29uY3VycmVudC9BY3Rvci5qYXZh)
 | `62.5000% <57.1429%> (-37.5000%)` | :arrow_down: |
   | 
[.../org/apache/groovy/runtime/async/DefaultActor.java](https://app.codecov.io/gh/apache/groovy/pull/2555?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fapache%2Fgroovy%2Fruntime%2Fasync%2FDefaultActor.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dyb292eS9ydW50aW1lL2FzeW5jL0RlZmF1bHRBY3Rvci5qYXZh)
 | `82.0513% <83.5294%> (-3.9487%)` | :arrow_down: |
   
   ... and [11 files with indirect coverage 
changes](https://app.codecov.io/gh/apache/groovy/pull/2555/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
   </details>
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :snowflake: [Test 
Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, 
report on failures, and find test suite problems.
   - :package: [JS Bundle 
Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save 
yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </details>




> groovy.concurrent.Actor: pre-GA hardening — Stop sentinel, error callback, 
> bounded mailbox, per-actor pool
> ----------------------------------------------------------------------------------------------------------
>
>                 Key: GROOVY-12033
>                 URL: https://issues.apache.org/jira/browse/GROOVY-12033
>             Project: Groovy
>          Issue Type: Improvement
>            Reporter: Paul King
>            Assignee: Paul King
>            Priority: Major
>             Fix For: 6.0.0-alpha-2
>
>
> h2. Summary
> Add four production-readiness improvements to {{groovy.concurrent.Actor}} 
> introduced in alpha-1:
> # {{Actor.Stop}} return-value sentinel for in-handler self-termination
> # {{onError}} callback so handler exceptions are no longer silently swallowed 
> for fire-and-forget {{send}}
> # Bounded mailbox with overflow strategy ({{BLOCK}} / {{DROP_NEWEST}} / 
> {{FAIL}})
> # Per-actor executor selection
> All changes are additive and backward compatible. Existing factory methods 
> and the existing {{ActorTest}} pass unchanged.
> h2. Public API additions
> h3. {{groovy.concurrent.Actor}}
> * {{Object Stop}} — public sentinel constant. A handler returning 
> {{Actor.Stop}} causes the actor to stop gracefully after the current message 
> (semantics of {{stop()}}: queued messages still drain). For {{sendAndGet}} 
> callers the reply is bound with the {{Stop}} sentinel itself; detect by 
> reference equality.
> * {{default Actor<T> onError(BiFunction<Throwable, ? super T, ?> handler)}} — 
> registers a handler invoked when message processing throws. Receives 
> {{(throwable, message)}}. Return value is a control signal: {{Actor.Stop}} 
> stops the actor, anything else continues. Default implementation throws 
> {{UnsupportedOperationException}}; {{DefaultActor}} overrides.
> * {{static Actor<T> reactor(Function<T,R> handler, ActorOptions options)}} — 
> new overload.
> * {{static Actor<T> stateful(S initial, BiFunction<S,T,S> handler, 
> ActorOptions options)}} — new overload.
> h3. New: {{groovy.concurrent.ActorOptions}} (record)
> {code:java}
> public record ActorOptions(int mailboxCapacity, Overflow overflow, Executor 
> executor) {
>     public enum Overflow { BLOCK, DROP_NEWEST, FAIL }
>     public static final ActorOptions DEFAULTS;
>     public ActorOptions withBoundedMailbox(int capacity, Overflow strategy);
>     public ActorOptions withExecutor(Executor executor);
>     public boolean isBounded();
> }
> {code}
> * {{mailboxCapacity == 0}} → unbounded (current behaviour).
> * {{executor == null}} → uses {{AsyncSupport.getExecutor()}} (current 
> behaviour).
> * Canonical constructor rejects negative capacity and null overflow.
> h2. Behavioural changes in {{org.apache.groovy.runtime.async.DefaultActor}}
> * Constructor now takes {{ActorOptions}}; queue is 
> {{LinkedBlockingQueue(capacity)}} when bounded.
> * {{send(T)}} routes through an {{enqueue(...)}} helper that honours the 
> overflow policy:
> ** {{BLOCK}} → {{queue.put}} (back-pressures the sending thread; on interrupt 
> restores the flag and throws a wrapping {{RuntimeException}}).
> ** {{DROP_NEWEST}} → {{queue.offer}}; on overflow the message is silently 
> dropped, and {{sendAndGet}} replies bind an {{IllegalStateException}} so 
> awaiters don't hang.
> ** {{FAIL}} → {{queue.offer}}; on overflow throws {{IllegalStateException}}. 
> {{sendAndGet}} additionally binds the reply with the same exception before 
> rethrowing.
> * {{processLoop}} checks {{result == Actor.Stop}} after the handler runs and 
> calls {{stop()}} (graceful — pending messages drain).
> * {{processLoop}}'s catch block now invokes the registered {{onError}} 
> handler if present, in addition to binding the failure on the reply for 
> {{sendAndGet}}. If the {{onError}} handler returns {{Actor.Stop}} the actor 
> stops. Exceptions from the {{onError}} handler itself are caught and 
> discarded so the processing loop cannot be destabilised.
> * {{StatefulProcessor.process}} preserves the prior state when the handler 
> returns {{Actor.Stop}}, so any messages queued behind the Stop-trigger still 
> observe real state during drain.
> * {{stop()}} now offers the poison pill on a possibly-bounded queue, falling 
> back to {{put}} so termination always succeeds.
> h2. Files touched
> || File || Change ||
> | {{src/main/java/groovy/concurrent/Actor.java}} | Modified — Stop sentinel, 
> onError default, 2 factory overloads, doc updates on {{send}} |
> | {{src/main/java/groovy/concurrent/ActorOptions.java}} | New |
> | {{src/main/java/org/apache/groovy/runtime/async/DefaultActor.java}} | 
> Modified — see behavioural changes above |
> | {{src/test/groovy/groovy/concurrent/ActorTest.groovy}} | Extended — 13 new 
> tests (see below) |
> h2. Tests
> 13 new tests, all passing alongside the existing 13:
> * {{testStatefulSelfStopsOnStopSentinel}}, 
> {{testReactorSelfStopsOnStopSentinel}}
> * {{testStopSentinelDrainsQueuedMessages}} — confirms FIFO drain after Stop
> * {{testStatePreservedAcrossStopSentinel}} — confirms StatefulProcessor 
> doesn't overwrite state with Stop
> * {{testOnErrorFiresForFireAndForgetException}}
> * {{testOnErrorAlsoFiresForSendAndGet}} — confirms both the {{Awaitable}} 
> failure path and the {{onError}} callback fire
> * {{testOnErrorReturningStopTerminatesActor}}
> * {{testOnErrorHandlerExceptionIsSwallowed}} — confirms a throwing 
> {{onError}} doesn't break the loop
> * {{testBoundedMailboxFailOverflowThrows}}
> * {{testBoundedMailboxDropNewest}}
> * {{testBoundedMailboxDropNewestReplyBindsError}} — overflow on 
> {{sendAndGet}} surfaces via the {{Awaitable}}
> * {{testBoundedMailboxBlockBackpressures}} — confirms the sender thread 
> actually parks until a slot frees
> * {{testPerActorExecutorIsUsed}} — confirms the handler runs on the supplied 
> executor's thread
> * {{testActorOptionsRejectsNegativeCapacity}}, 
> {{testActorOptionsWithBoundedMailboxRejectsZero}}
> Full suite of 135 tests across the 18 {{groovy.concurrent.*}} test classes 
> still passes.
> h2. Out of scope / deferred
> Deferred to a future ticket (proposed for Groovy 7):
> * Idle / receive timeout ({{react(timeout)}} from GPars, 
> {{setReceiveTimeout}} from Pekko)
> * Restart-with-backoff supervision policy
> * Per-actor {{withTimers}} scheduling hub
> * Stash / unstash
> * {{become}}-style behaviour swap
> Not adopted (out of scope for {{groovy.concurrent}} entirely): actor 
> hierarchy with supervision trees, remote actors, persistence, sharding. These 
> are Pekko's domain; {{groovy.concurrent.Actor}} is for serialising in-process 
> mutable state.
> h2. Compatibility
> * Source compatible: no existing signature on the {{Actor}} interface or 
> {{DefaultActor}} factory methods changed.
> * Binary compatible for all existing call sites; existing 2-arg factories 
> route through {{ActorOptions.DEFAULTS}}.
> * {{onError}} is a {{default}} method, so existing implementors of {{Actor}} 
> compile unchanged (calls to {{onError}} on a non-{{DefaultActor}} 
> implementation throw {{UnsupportedOperationException}}).
> h2. Notes for follow-up
> While adding tests, two Groovy 6 {{await}} parser-sugar quirks surfaced and 
> are worth filing separately (not part of this change):
> # {{await\(x).is(y)}} parses as {{await(x.is(y))}} — the postfix chain after 
> {{await(...)}} is consumed into its argument list.
> # {{await\(x)}} as a standalone statement is rejected with "Modifiers or 
> return type is required" — {{await}} requires expression context.
> Both worked around in the tests by binding {{def v = await(...)}} first.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to