Skip to content

feat: Allow custom EventLoopGroup and Timeout#78

Merged
djones6 merged 16 commits intomasterfrom
issue.eventLoopThreads
Nov 5, 2019
Merged

feat: Allow custom EventLoopGroup and Timeout#78
djones6 merged 16 commits intomasterfrom
issue.eventLoopThreads

Conversation

@djones6
Copy link
Contributor

@djones6 djones6 commented Oct 23, 2019

This PR:

  • Updates the default EventLoopGroup to use 1 thread per available processor,
  • Permits customization of the ELG via the eventLoopGroup init parameter,
  • Adds an optional timeout: HTTPClient.Configuration.Timeout parameter to RestRequest.init(),
  • Adds tests for request timeout and customization of the EventLoopGroup.

Also updates the version of async-http-client to 1.0.0 (resolves #79)

Motivation

RestRequest currently uses a global EventLoopGroup containing a single thread. This is sufficient for simple applications, however for highly parallel applications it creates a bottleneck.

Also, there is currently no way to configure the timeout for requests, and HTTPClient's default is to set no timeouts. This could leave an application stuck indefinitely.

return result
}
set {
_globalELG = newValue
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks racey....

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about b96f186

@djones6 djones6 changed the title feat: Allow access to EventLoopGroup feat: Allow access to EventLoopGroup and Timeout Oct 23, 2019
@djones6 djones6 requested a review from ianpartridge November 4, 2019 09:37
...to avoid naming clash with Kitura-NIO
}

fileprivate static var _globalELG: EventLoopGroup?
fileprivate static var _globalELGIsSet: Atomic<Int> = .init(value: 0)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is not on a hot path, so just use a DispatchQueue

return _globalELGSetter.sync {
if let result = _globalELG { return result }
#if os(Linux)
let numberOfCores = Int(linux_sched_getaffinity_sr())
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably overkill for SwiftyRequest, just use System.coreCount for now

@djones6 djones6 changed the title feat: Allow access to EventLoopGroup and Timeout feat: Allow custom EventLoopGroup and Timeout Nov 5, 2019
@djones6 djones6 merged commit 34cf8de into master Nov 5, 2019
@djones6 djones6 deleted the issue.eventLoopThreads branch November 18, 2019 15:58
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.

should use async-http-client from 1.0.0

2 participants