Skip to content

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

Merged
cheenamalhotra merged 3 commits intomicrosoft:devfrom
robertonr:dev
Aug 17, 2018
Merged

Improve performance of readLong function unrolling loop and using bitwise operators instead of additions#763
cheenamalhotra merged 3 commits intomicrosoft:devfrom
robertonr:dev

Conversation

@robertonr
Copy link
Copy Markdown
Contributor

@robertonr robertonr commented Jul 30, 2018

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
Copy Markdown

codecov-io commented Jul 30, 2018 •
edited
Loading

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.

Improve performance of readLong function unrolling loop and using bit…
e52a793 
…wise

operators instead of additions + unit test
@msftclas
Copy link
Copy Markdown

msftclas commented Aug 5, 2018 •
edited
Loading

CLA assistant check
All CLA requirements met.

@cheenamalhotra cheenamalhotra added this to the 7.1.0 milestone Aug 16, 2018
@rene-ye
Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

robertonr commented Aug 16, 2018 via email •
edited
Loading

@cheenamalhotra cheenamalhotra merged commit 2c86309 into microsoft:dev Aug 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cheenamalhotra cheenamalhotra cheenamalhotra approved these changes

+3 more reviewers

@rene-ye rene-ye rene-ye approved these changes

@ulvii ulvii ulvii approved these changes

@peterbae peterbae peterbae approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

7.1.0

Development

Successfully merging this pull request may close these issues.

7 participants

@robertonr @codecov-io @msftclas @rene-ye @cheenamalhotra @ulvii @peterbae