Skip to content

Performance | Remove Enum.values() calls to avoid unnecessary array cloning #1065

Merged
ulvii merged 2 commits intomicrosoft:devfrom
ulvii:Issue1051
May 28, 2019
Merged

Performance | Remove Enum.values() calls to avoid unnecessary array cloning #1065
ulvii merged 2 commits intomicrosoft:devfrom
ulvii:Issue1051

Conversation

@ulvii
Copy link
Copy Markdown
Contributor

@ulvii ulvii commented May 22, 2019

Replacing Enum.values() calls with static arrays. Fix for #1051

@ulvii
Copy link
Copy Markdown
Contributor Author

ulvii commented May 22, 2019

Hi @cogman,

I created a PR with your suggestions. Would you mind testing the attached jars and let me know whether the fix resolves your issue?
PR1065_jars.zip

@ulvii ulvii requested review from cheenamalhotra, lilgreenbird, peterbae and rene-ye and removed request for cheenamalhotra May 22, 2019 18:02
@codecov-io
Copy link
Copy Markdown

codecov-io commented May 22, 2019 •
edited
Loading

Codecov Report

Merging #1065 into dev will increase coverage by 0.08%.
The diff coverage is 74.19%.

Impacted file tree graph

@@             Coverage Diff              @@
##                dev    #1065      +/-   ##
============================================
+ Coverage     53.08%   53.16%   +0.08%     
- Complexity     3159     3187      +28     
============================================
  Files           119      119              
  Lines         28022    28089      +67     
  Branches       4688     4710      +22     
============================================
+ Hits          14875    14934      +59     
- Misses        10912    10918       +6     
- Partials       2235     2237       +2
Impacted Files Coverage Δ Complexity Δ
...rosoft/sqlserver/jdbc/SQLServerEncryptionType.java 68.75% <50%> (+2.08%) 5 <2> (ø) ⬇️
...rosoft/sqlserver/jdbc/InternalSpatialDatatype.java 91.66% <50%> (+0.36%) 6 <2> (ø) ⬇️
...n/java/com/microsoft/sqlserver/jdbc/DataTypes.java 80.3% <77.77%> (+0.22%) 6 <0> (ø) ⬇️
...rc/main/java/com/microsoft/sqlserver/jdbc/DDC.java 47.51% <0%> (-0.87%) 111% <0%> (-1%)
...c/main/java/com/microsoft/sqlserver/jdbc/Util.java 59.69% <0%> (-0.44%) 90% <0%> (-1%)
...in/java/com/microsoft/sqlserver/jdbc/IOBuffer.java 61.2% <0%> (-0.3%) 0% <0%> (ø)
...om/microsoft/sqlserver/jdbc/SQLServerBulkCopy.java 51.56% <0%> (-0.07%) 255% <0%> (-1%)
...m/microsoft/sqlserver/jdbc/SQLServerResultSet.java 44.25% <0%> (+0.07%) 328% <0%> (+1%) ⬆️
.../microsoft/sqlserver/jdbc/SQLServerConnection.java 49.75% <0%> (+0.13%) 397% <0%> (+2%) ⬆️
...rc/main/java/com/microsoft/sqlserver/jdbc/TVP.java 44.53% <0%> (+0.8%) 26% <0%> (ø) ⬇️
... and 2 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 b91c7ea...b677081. Read the comment docs.

@cheenamalhotra cheenamalhotra added this to the 7.3.1 milestone May 22, 2019
@cogman
Copy link
Copy Markdown
Contributor

cogman commented May 24, 2019

@ulvii Sorry for the delay, but it looks good to me.

I created a quick test app to push in 1000 TVP entries 1000 times. These are the results I got

7.2.2, 1.17gb of allocations
The preview, 447Mb of allocations

CPU utilization was also much lower throughout the second run.

The following file is flight recorder results taken through my test run
https://github.com/cogman/demo/blob/fasterGrouping/sqltest.zip

@ulvii ulvii changed the title Fix | Remove Enum.values() calls Performance | Remove Enum.values() calls to avoid unnecessary array cloning May 27, 2019
@ulvii ulvii merged commit 8476a46 into microsoft:dev May 28, 2019
@ulvii ulvii deleted the Issue1051 branch May 28, 2019 00:53
@ulvii ulvii mentioned this pull request May 28, 2019
Closed
@ulvii ulvii mentioned this pull request Jun 12, 2020
Merged
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

@lilgreenbird lilgreenbird Awaiting requested review from lilgreenbird

+2 more reviewers

@rene-ye rene-ye rene-ye 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.3.1

Development

Successfully merging this pull request may close these issues.

6 participants

@ulvii @codecov-io @cogman @rene-ye @cheenamalhotra @peterbae