-
Notifications
You must be signed in to change notification settings - Fork 356
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
Conversation
#### 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]; |
There was a problem hiding this comment.
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;
}
There was a problem hiding this comment.
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
@stevegury does this look ok now? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
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.
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.
Problem
TimeoutPublisher
creates a newTimeoutException
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.