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

Improve performance of readLong function unrolling loop and using bitwise operators instead of additions #763

Merged
merged 3 commits into from
Aug 17, 2018

Conversation

robertonr
Copy link
Contributor

Hello, I'm using mssql-jdbc driver for long time and profiling my application for 20 minutes I found that TDSChannel.read() is taking around 548297 milliseconds of CPU (I guess it's due to synchronization). However improving readLong method I reduced around 30000 milliseconds of pure CPU usage.

Could you merge this small change, please?

@codecov-io
Copy link

codecov-io commented Jul 30, 2018

Codecov Report

Merging #763 into dev will increase coverage by 0.24%.
The diff coverage is 90%.

Impacted file tree graph

@@             Coverage Diff              @@
##                dev     #763      +/-   ##
============================================
+ Coverage     48.17%   48.41%   +0.24%     
- Complexity     2776     2799      +23     
============================================
  Files           116      116              
  Lines         27854    27852       -2     
  Branches       4636     4635       -1     
============================================
+ Hits          13418    13484      +66     
- Misses        12215    12232      +17     
+ Partials       2221     2136      -85
Flag Coverage Δ Complexity Δ
#JDBC42 47.54% <90%> (-0.17%) 2723 <2> (-12)
#JDBC43 48.31% <90%> (+0.3%) 2798 <2> (+29) ⬆️
Impacted Files Coverage Δ Complexity Δ
...in/java/com/microsoft/sqlserver/jdbc/IOBuffer.java 55.7% <0%> (+1.89%) 0 <0> (ø) ⬇️
...c/main/java/com/microsoft/sqlserver/jdbc/Util.java 60.9% <100%> (+1.08%) 90 <2> (+3) ⬆️
...rc/main/java/com/microsoft/sqlserver/jdbc/DDC.java 44.63% <0%> (-1.1%) 106% <0%> (-2%)
...ncurrentlinkedhashmap/ConcurrentLinkedHashMap.java 38.36% <0%> (-0.65%) 42% <0%> (-1%)
...rc/main/java/com/microsoft/sqlserver/jdbc/dtv.java 62.31% <0%> (-0.24%) 0% <0%> (ø)
...m/microsoft/sqlserver/jdbc/SQLServerResultSet.java 32.03% <0%> (-0.2%) 247% <0%> (-1%)
...m/microsoft/sqlserver/jdbc/SQLServerException.java 77.27% <0%> (ø) 32% <0%> (+1%) ⬆️
...om/microsoft/sqlserver/jdbc/SQLServerBulkCopy.java 52.6% <0%> (ø) 262% <0%> (ø) ⬇️
...n/java/com/microsoft/sqlserver/jdbc/DataTypes.java 79.48% <0%> (+0.19%) 5% <0%> (+1%) ⬆️
...m/microsoft/sqlserver/jdbc/SQLServerStatement.java 60.05% <0%> (+0.28%) 142% <0%> (+6%) ⬆️
... and 5 more

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 e301555...310a52f. Read the comment docs.

…wise

operators instead of additions + unit test
@msftclas
Copy link

msftclas commented Aug 5, 2018

CLA assistant check
All CLA requirements met.

@cheenamalhotra cheenamalhotra added this to the 7.1.0 milestone Aug 16, 2018
@rene-ye
Copy link
Member

rene-ye commented Aug 16, 2018

Hi @robertonr, thank you for your contribution to the mssql-jdbc driver. Please restrict the visibility of writeLong as to not introduce any new public APIs. If the concern is allowing the function to be tested, the team can do that. Other than that the changes look good.

@robertonr
Copy link
Contributor Author

robertonr commented Aug 16, 2018 via email

@cheenamalhotra cheenamalhotra merged commit 2c86309 into microsoft:dev Aug 17, 2018
MSSQL JDBC automation moved this from Under Peer Review to Closed/Merged PRs Aug 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
MSSQL JDBC
  
Closed/Merged PRs
Development

Successfully merging this pull request may close these issues.

None yet

7 participants