Skip to content
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

performance optimization by Interleave #811

Merged
merged 2 commits into from
Jun 30, 2017
Merged

performance optimization by Interleave #811

merged 2 commits into from
Jun 30, 2017

Conversation

1178615156
Copy link

Problem

query : ctx.run(table.filter(e => liftQuery(ids).contains(e.id)))
it will take long long time when ids size is big (E.g 100000)

Solution

scala collection complexity
because for List ; prepend is O(1) , append is O(n)
so use prepend replace append is more effectively

Checklist

  • Unit test all changes
  • Update README.md if applicable
  • Add [WIP] to the pull request title if it's work in progress
  • Squash commits that aren't meaningful changes
  • Run sbt scalariformFormat test:scalariformFormat to make sure that the source files are formatted

@getquill/maintainers

@codecov-io
Copy link

Codecov Report

Merging #811 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #811   +/-   ##
=======================================
  Coverage   93.51%   93.51%           
=======================================
  Files         134      134           
  Lines        2514     2514           
  Branches       37       33    -4     
=======================================
  Hits         2351     2351           
  Misses        163      163
Impacted Files Coverage Δ
...e/src/main/scala/io/getquill/util/Interleave.scala 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1705dca...a8fc2f1. Read the comment docs.

@jilen
Copy link
Collaborator

jilen commented Jun 29, 2017

I am afraid reverse is also O(n)

@1178615156
Copy link
Author

1178615156 commented Jun 29, 2017

@jilen yes reverse is O(n)
but the complexity of the new method is :
O(n)(one iteration) * O(1)(prepend) + O(n)(reverse) + O(n)(append) = O(n)

before is O(n)(one iteration) * O(n)(append) = O(n^2)

Copy link
Collaborator

@mosyp mosyp left a comment

Choose a reason for hiding this comment

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

Great contribution, thank you!

@1178615156 1178615156 closed this Jun 30, 2017
@1178615156 1178615156 deleted the performance-optimization branch June 30, 2017 07:43
@mxl
Copy link
Contributor

mxl commented Jun 30, 2017

@1178615156 We haven't merged PR yet. Please reopen it.

@1178615156 1178615156 restored the performance-optimization branch June 30, 2017 08:35
@1178615156 1178615156 reopened this Jun 30, 2017
@mosyp mosyp merged commit f3b23d6 into zio:master Jun 30, 2017
@1178615156 1178615156 deleted the performance-optimization branch June 30, 2017 09:09
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.

6 participants