Skip to content

Empty stacktrace of TimeoutException #202

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

Merged
merged 4 commits into from
Dec 9, 2016

Conversation

NiteshKant
Copy link
Contributor

Problem

TimeoutPublisher creates a new TimeoutException instance for every timeout. This is costly when the system is under load and has a lot of timers that timeout, which is typical when handling latent outbound services.

Modification

Used a cached TimeoutException instance and empty the stacktrace of the exception.

Result

Less overhead of timeouts.

#### Problem

`TimeoutPublisher` creates a new `TimeoutException` instance for every timeout. This is costly when the system is under load and has a lot of timers that timeout, which is typical when handling latent outbound services.

#### Modification

Used a cached `TimeoutException` instance and empty the stacktrace of the exception.

#### Result

Less overhead of timeouts.
@SuppressWarnings("ThrowableInstanceNeverThrown")
private static final TimeoutException timeoutException = new TimeoutException();

private static final StackTraceElement[] EMPTY_STACK = new StackTraceElement[0];
Copy link
Member

Choose a reason for hiding this comment

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

Rather than setting the stack trace manually, it would be better to override the fillInStacktrace method in all reactivesocket exceptions.

public synchronized Throwable fillInStackTrace() {
    return this;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense. I will update.
BTW, this code is in publishers package and TimeoutException is in core, so this code here uses java.util.concurrent.TimeoutException

@NiteshKant
Copy link
Contributor Author

@stevegury does this look ok now?

Copy link
Member

@stevegury stevegury left a comment

Choose a reason for hiding this comment

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

👍

@NiteshKant NiteshKant merged commit bc9c3e6 into rsocket:0.5.x Dec 9, 2016
@NiteshKant NiteshKant deleted the 0.5.x-timeout-ex branch December 9, 2016 23:24
NiteshKant added a commit to NiteshKant/reactivesocket-java that referenced this pull request Dec 14, 2016
Empty stacktrace of `TimeoutException`

#### Problem

`TimeoutPublisher` creates a new `TimeoutException` instance for every timeout. This is costly when the system is under load and has a lot of timers that timeout, which is typical when handling latent outbound services.

#### Modification

Used a cached `TimeoutException` instance and empty the stacktrace of the exception.

#### Result

Less overhead of timeouts.
ilayaperumalg pushed a commit to ilayaperumalg/rsocket-java that referenced this pull request Dec 26, 2017
Improving the sequence and lifetime docs for channel, and somewhat stream.
Specifically this clarifies that COMPLETE is needed in both directions for channel, and how it behaves with ERROR and CANCEL.
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