Skip to content

Conversation

@magicprinc
Copy link
Contributor

ChannelClosedException isn't created directly and only used as a parent, so it should be an abstract class 🙏

Channel.isUnlimited is redundant: extra byte and extra if condition

Replace Math.ceil with faster integer division

1) delete redundant field isUnlimited (-1 byte; simpler if)
2) integer division
@magicprinc
Copy link
Contributor Author

PS: some ifs can be replaced with switch: It could be faster 🤷‍♀️
Like

if (state == INTERRUPTED_SEND) {
    // cell interrupted -> trying with a new one
    return ReceiveResult.FAILED;
} else if (state == RESUMING) {
    // expandBuffer() is resuming the sender -> repeat
    Thread.onSpinWait();
} else if (state == CLOSED) {
    return ReceiveResult.CLOSED;
} else {
    throw new IllegalStateException(
            "Unexpected state: " + state + " in channel: " + this);
}

to

switch (state){
case CellState.INTERRUPTED_SEND -> {
    // cell interrupted -> trying with a new one
    return ReceiveResult.FAILED;
}
case CellState.RESUMING -> Thread.onSpinWait();// expandBuffer() is resuming the sender -> repeat
case CellState.CLOSED -> {
    return ReceiveResult.CLOSED;
}
default -> throw new IllegalStateException(
        "Unexpected state: " + state + " in channel: " + this);
}

@magicprinc
Copy link
Contributor Author

I have a litte bit more intrusive https://github.com/magicprinc/jox/commits/feature/more-optimizations/

  • delete reduntant DefaultClauseMarker
  • fix test on windows

I can push it here if you like

@magicprinc
Copy link
Contributor Author

ArrayBlockingQueue -vs- Channel.newBufferedChannel(1000)
ABQ is clear winner 😭

newUnlimitedChannel + https://github.com/magicprinc/jox/commits/feature/more-optimizations/
Channel is winner

BUT .size() is required, to protect from queue blowing up and OOM

@adamw
Copy link
Member

adamw commented Nov 13, 2025

I'm afraid we cannot change ChannelClosedException. Does it have any performance impact?

Error:  Failed to execute goal com.github.siom79.japicmp:japicmp-maven-plugin:0.24.2:cmp (default) on project channels: There is at least one incompatibility: com.softwaremill.jox.ChannelClosedException:CLASS_NOW_ABSTRACT -> [Help 1]

The other changes look good :)

We could also do switch instead of ifs, it would also be more readable. But probably better as a separate PR.

@magicprinc
Copy link
Contributor Author

Does it have any performance impact?

Pure abstract beauty

Rolled back ✅

@magicprinc
Copy link
Contributor Author

magicprinc commented Nov 13, 2025

done ✅
Please review 🙏

——
Sorry, forgot about separate PR :(

I can push commits into separate PRs…

@adamw adamw merged commit 0f87925 into softwaremill:main Nov 14, 2025
9 checks passed
@adamw
Copy link
Member

adamw commented Nov 14, 2025

Thank you :)

@magicprinc
Copy link
Contributor Author

magicprinc commented Nov 14, 2025

Thank You for such amazing project!

I really think it deserves the honor of joining the JDK.

As for communications (openness, friendliness): You are the Nr.1 open source project author, that I have ever met 🤝💖

@adamw
Copy link
Member

adamw commented Nov 14, 2025

Thank You for such amazing project!
I really think it deserves the honor of joining the JDK.

Well remember that it's just implementing an algorithm described and used in Kotlin :)

As for communications (openness, friendliness): You are the Nr.1 open source project author, that I have ever met 🤝💖

😅 now I have to try and keep up ;)

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.

2 participants