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

Inserts into varbinary(max) col with FILESTREAM or reads from that column takes 10's of seconds #971

Closed
sammaniamsam opened this issue Sep 18, 2019 · 10 comments · Fixed by #1066
Labels
discussion enhancement Follow up Recent question asked by tedious members for old issues or discussion that requires member input released

Comments

@sammaniamsam
Copy link

sammaniamsam commented Sep 18, 2019

For some reason, inserts into varbinary(max) col with FILESTREAM or reads from that column takes 10's of seconds with tedious. I took the time to trace the bottle neck in my node application back to tedious. I tested the following query using roughly two 25MB files.

select * from documents where file_extension = 'zip'

MSSQL Management Studio

Screen Shot 2019-09-18 at 1 49 36 PM

Chrome Dev Tools

Screen Shot 2019-09-18 at 1 53 16 PM

Code snippet (not complete)

tedious-snippet.txt

@IanChokS
Copy link
Member

Hi @sammaniamsam,

Thanks for pointing this out. I think this may be due to how token parsing is implemented in Tedious, namely there's a lot of asynchronous call backs that take up a lot of memory which becomes more so when using varbinary(max) and perhaps varchar(max) as well. We do have a plan to refactor how things are implemented in Tedious to increase performance coming up in the near future.

What are your thoughts @arthurschreiber, @MichaelSun90 ?

@IanChokS
Copy link
Member

Hi @sammaniamsam, I am just curious what version of Tedious are you using that causes this performance bottleneck? #1006

@sammaniamsam
Copy link
Author

@IanChokS I'm using "^5.0.3"

@IanChokS IanChokS added discussion Follow up Recent question asked by tedious members for old issues or discussion that requires member input labels Dec 13, 2019
@IanChokS
Copy link
Member

IanChokS commented Dec 18, 2019

Ok after sifting through all of the currently open issues, it's clear that performance blockage is a common problem (e.g., #879, #781,#475,#467, #319, #303). Currently we have on our road map things like implementing Always Encrypted feature, improved data type validation / conversion, Pluggable authentication providers, and refactoring current data types. But based on the issues being raised, it seems like this performance block is the most common barrier people are facing when using tedious. I'm discussing with Arthur and the team on what work needs to be prioritized, so if you feel like improving performance is the biggest change you'd like to see, please leave a like or comment your thoughts about how we should move forward. Any feedback would be of great help! 🙇

@IanChokS
Copy link
Member

IanChokS commented Jan 17, 2020

WIP to fix performance issues -> #1037

Description: #1038

@IanChokS
Copy link
Member

IanChokS commented Mar 3, 2020

Hi @sammaniamsam, we've recently merged #1049, #1044, #1037 that hopefully improves performance. Do you mind if you run your benchmark again against the latest master branch and let us know if you see an improve in performance from your end?

Thanks! 🙇

edit: it hasn't been released yet on npm, but the changes are in the current master branch

@sammaniamsam
Copy link
Author

@IanChokS Absolutely! Thank you guys for taking care of this. I look forward to testing the new improvements.

@sammaniamsam
Copy link
Author

sammaniamsam commented Mar 6, 2020

@IanChokS The results are in the table below. Let me know if you have any questions.

Method Tedious Version Number of Files Size Time to Execute (ms)
Read ^6.3.0 1 23.33 MB 2435.834
Read ^6.3.0 2 46.66 MB 5008.435
Read ^6.3.0 3 136.5 KB 329.727
Insert ^6.3.0 2 11.9 MB 4316.95
Insert ^6.3.0 4 59 KB 42.864
Read Latest Master 1 23.33 MB 2771.478
Read Latest Master 2 46.66 MB 4877.394
Read Latest Master 3 136.5 KB 93.575
Insert Latest Master 2 11.9 MB 2535.267
Insert Latest Master 4 59 KB 43.886

Screen Shot 2020-03-06 at 1 32 17 PM
Screen Shot 2020-03-06 at 1 31 18 PM

@IanChokS
Copy link
Member

IanChokS commented Mar 6, 2020

That's looking good! It matches our results for inserts. For e.g, memory usage has dropped by half.
image
image

We'll definitely be looking to increase read time as well in the future 💪

@arthurschreiber
Copy link
Collaborator

🎉 This issue has been resolved in version 8.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion enhancement Follow up Recent question asked by tedious members for old issues or discussion that requires member input released
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants