Skip to content

Stack safety of combineRequests - #227

Merged
purrgrammer merged 3 commits into
xebia-functional:masterfrom
abestel:feature/stack_safety
Oct 25, 2019
Merged

Stack safety of combineRequests#227
purrgrammer merged 3 commits into
xebia-functional:masterfrom
abestel:feature/stack_safety

Conversation

@abestel

@abestel abestel commented Oct 25, 2019

Copy link
Copy Markdown

Context
Our application crashed because of a StackOverflowError in combineRequests.
We noticed it because we run the application with -Xss256k.

Proposal
Trampoline combineRequests and make it stack safe.

Side note
During my tests, I noticed that a big batch (5000 elements) was extremely slow to be processed.
Most of the time is spent into combineIdentities (half in NonEmptyList.exists and the other half in NonEmptyList.:+).
By changing FetchQuery to return identities as a Set[I], the code is much faster (the big batch test went from minutes to seconds).
Any objection?

@purrgrammer

Copy link
Copy Markdown
Contributor

During my tests, I noticed that a big batch (5000 elements) was extremely slow to be processed.
Most of the time is spent into combineIdentities (half in NonEmptyList.exists and the other half in NonEmptyList.:+).

NonEmptyList is not the right data structure to use internally, I was aware this could be a problem so thanks for actually fixing it!

By changing FetchQuery to return identities as a Set[I], the code is much faster (the big batch test went from minutes to seconds).

🚀

The changes look good, I'll merge and cut a new release. Thanks a lot for your time!

@purrgrammer
purrgrammer merged commit 2bdd2e3 into xebia-functional:master Oct 25, 2019
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