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

Perform memory profiling and comparison benchmarking for pull requests (FF-1563) #32

Merged
merged 1 commit into from
Feb 15, 2024

Conversation

leoromanovsky
Copy link
Member

@leoromanovsky leoromanovsky commented Feb 14, 2024

context

our customers expect the golang sdk to be performant and have minimal impact on their running server.

as we are making changes to the golang sdk, we need evaluate those changes for correctness. with this benchmark we will have added information to see about additional memory allocations happening with the proposed branch.

mechanism of action

  • checks out main branch
  • executes make profile-memory to produce memprofile.main.out (binary) and memprofile.main.text (human readible)
 ➜ cat memprofile.main.text
Type: alloc_space
Time: Feb 14, 2024 at 3:54pm (PST)
Showing nodes accounting for 31134.62kB, 100% of 31134.62kB total
Showing top 50 nodes out of 66
      flat  flat%   sum%        cum   cum%
 8193.70kB 26.32% 26.32% 12801.91kB 41.12%  encoding/json.mapEncoder.encode
 3584.20kB 11.51% 37.83%  3584.20kB 11.51%  internal/reflectlite.Swapper
 2049.16kB  6.58% 44.41% 14851.07kB 47.70%  encoding/json.Marshal
 2048.41kB  6.58% 50.99%  2048.41kB  6.58%  os.statNolog
 2048.16kB  6.58% 57.57%  2048.16kB  6.58%  strings.genSplit
    1799kB  5.78% 63.35%  7431.83kB 23.87%  github.com/stretchr/testify/mock.(*Mock).MethodCalled
 1536.61kB  4.94% 68.28%  1536.61kB  4.94%  reflect.mapassign_faststr0
 1536.21kB  4.93% 73.22%  4096.86kB 13.16%  encoding/json.Unmarshal
 1039.13kB  3.34% 76.55%  1039.13kB  3.34%  io.ReadAll
 1024.16kB  3.29% 79.84%  1024.16kB  3.29%  fmt.Sprintf
 1024.02kB  3.29% 83.13%  1024.02kB  3.29%  reflect.copyVal
  636.73kB  2.05% 85.18%   636.73kB  2.05%  github.com/stretchr/testify/mock.(*Mock).calls (inline)
  518.65kB  1.67% 86.84% 31134.62kB   100%  github.com/Eppo-exp/golang-sdk/v2/eppoclient.Test_e2e
  512.16kB  1.64% 88.49%   512.16kB  1.64%  regexp/syntax.(*compiler).inst (inline)
  512.11kB  1.64% 90.13%  5120.70kB 16.45%  github.com/stretchr/testify/assert.CallerInfo
  512.06kB  1.64% 91.78% 27916.03kB 89.66%  github.com/Eppo-exp/golang-sdk/v2/eppoclient.(*EppoClient).getAssignment
  512.06kB  1.64% 93.42%  8968.11kB 28.80%  github.com/Eppo-exp/golang-sdk/v2/eppoclient.(*mockLogger).LogAssignment
  512.05kB  1.64% 95.07%   512.05kB  1.64%  regexp/syntax.(*parser).newRegexp (inline)
  512.02kB  1.64% 96.71%   512.02kB  1.64%  reflect.makemap
  512.02kB  1.64% 98.36%   512.02kB  1.64%  encoding/hex.EncodeToString (inline)
  512.01kB  1.64%   100%  2048.22kB  6.58%  encoding/json.(*decodeState).literalStore
         0     0%   100%  2048.22kB  6.58%  encoding/json.(*decodeState).array
         0     0%   100%  4096.86kB 13.16%  encoding/json.(*decodeState).object
         0     0%   100%  4096.86kB 13.16%  encoding/json.(*decodeState).unmarshal
         0     0%   100%  4096.86kB 13.16%  encoding/json.(*decodeState).value
         0     0%   100% 12801.91kB 41.12%  encoding/json.(*encodeState).marshal
         0     0%   100% 12801.91kB 41.12%  encoding/json.(*encodeState).reflectValue
         0     0%   100%  8705.17kB 27.96%  encoding/json.arrayEncoder.encode
         0     0%   100% 10241.47kB 32.89%  encoding/json.interfaceEncoder
         0     0%   100%  8705.17kB 27.96%  encoding/json.sliceEncoder.encode
         0     0%   100% 27916.03kB 89.66%  github.com/Eppo-exp/golang-sdk/v2/eppoclient.(*EppoClient).GetStringAssignment (inline)
         0     0%   100%  8968.11kB 28.80%  github.com/Eppo-exp/golang-sdk/v2/eppoclient.(*EppoClient).getAssignment.func1
         0     0%   100%  1536.21kB  4.93%  github.com/Eppo-exp/golang-sdk/v2/eppoclient.(*Value).UnmarshalJSON
         0     0%   100% 17411.71kB 55.92%  github.com/Eppo-exp/golang-sdk/v2/eppoclient.(*configurationStore).GetConfiguration
         0     0%   100% 17411.71kB 55.92%  github.com/Eppo-exp/golang-sdk/v2/eppoclient.(*experimentConfigurationRequestor).GetConfiguration
         0     0%   100%   512.02kB  1.64%  github.com/Eppo-exp/golang-sdk/v2/eppoclient.getShard
         0     0%   100%  2063.21kB  6.63%  github.com/Eppo-exp/golang-sdk/v2/eppoclient.getTestData
         0     0%   100%  2063.21kB  6.63%  github.com/Eppo-exp/golang-sdk/v2/eppoclient.initFixture
         0     0%   100%   512.02kB  1.64%  github.com/Eppo-exp/golang-sdk/v2/eppoclient.isInExperimentSample
         0     0%   100%   636.73kB  2.05%  github.com/stretchr/testify/mock.(*Mock).AssertCalled
         0     0%   100%  8456.05kB 27.16%  github.com/stretchr/testify/mock.(*Mock).Called
         0     0%   100%   512.14kB  1.64%  github.com/stretchr/testify/mock.(*Mock).findExpectedCall
         0     0%   100%   636.73kB  2.05%  github.com/stretchr/testify/mock.(*Mock).methodWasCalled
         0     0%   100%   512.14kB  1.64%  github.com/stretchr/testify/mock.Arguments.Diff
         0     0%   100%  2048.41kB  6.58%  os.Getwd
         0     0%   100%  2048.41kB  6.58%  path/filepath.Abs
         0     0%   100%  2048.41kB  6.58%  path/filepath.abs (inline)
         0     0%   100%  2048.41kB  6.58%  path/filepath.unixAbs
         0     0%   100%  1024.02kB  3.29%  reflect.(*MapIter).Key
         0     0%   100%   512.02kB  1.64%  reflect.MakeMap (inline)
  • checks out the proposed pull request branch and proposed a corresponding memprofile.feat.out and memprofile.feat.text files
  • produces a comparison of the files to determine if memory allocations were added or removed with the new code make profile-memory-compare BASE_FILE=memprofile1.out FEAT_FILE=memprofile2.out
➜  eppoclient git:(main) ✗ cat comparison.text
Type: alloc_space
Time: Feb 14, 2024 at 12:14pm (PST)
Showing nodes accounting for -8133.77kB, 14.83% of 54863.67kB total
Dropped 5 nodes (cum <= 274.32kB)
      flat  flat%   sum%        cum   cum%
-4096.61kB  7.47%  7.47% -7680.72kB 14.00%  encoding/json.mapEncoder.encode
 3584.71kB  6.53%  0.93%  4096.73kB  7.47%  os.statNolog
-3072.34kB  5.60%  6.53% -3072.34kB  5.60%  strings.genSplit
-2561.31kB  4.67% 11.20% -10242.04kB 18.67%  encoding/json.Marshal
-2048.03kB  3.73% 14.93% -2048.03kB  3.73%  reflect.copyVal
-1536.09kB  2.80% 17.73% -1536.09kB  2.80%  internal/reflectlite.Swapper
 1024.23kB  1.87% 15.87%  1024.23kB  1.87%  regexp/syntax.(*compiler).inst (inline)
-1024.14kB  1.87% 17.73% -2540.39kB  4.63%  encoding/json.Unmarshal
  536.37kB  0.98% 16.76%  1561.51kB  2.85%  github.com/stretchr/testify/mock.(*Mock).MethodCalled
  520.04kB  0.95% 15.81%   520.04kB  0.95%  io.ReadAll
  518.65kB  0.95% 14.86% -8138.42kB 14.83%  github.com/Eppo-exp/golang-sdk/v2/eppoclient.Test_e2e
  512.75kB  0.93% 13.93%   512.75kB  0.93%  sync.(*Pool).pinSlow
 -512.20kB  0.93% 14.86%  -512.20kB  0.93%  reflect.mapassign_faststr0
  512.11kB  0.93% 13.93%  3097.86kB  5.65%  github.com/stretchr/testify/mock.(*Mock).Called
  512.06kB  0.93% 13.00%  3609.92kB  6.58%  github.com/Eppo-exp/golang-sdk/v2/eppoclient.(*mockLogger).LogAssignment
 -512.04kB  0.93% 13.93% -13826.52kB 25.20%  github.com/Eppo-exp/golang-sdk/v2/eppoclient.(*configurationStore).GetConfiguration
  512.03kB  0.93% 13.00%   512.03kB  0.93%  github.com/Eppo-exp/golang-sdk/v2/eppoclient.getShard
  512.03kB  0.93% 12.06%   512.03kB  0.93%  time.Time.String
 -512.02kB  0.93% 13.00% -10728.84kB 19.56%  github.com/Eppo-exp/golang-sdk/v2/eppoclient.(*EppoClient).getAssignment
 -512.02kB  0.93% 13.93%  -512.02kB  0.93%  reflect.makemap
  512.02kB  0.93% 13.00%   512.02kB  0.93%  syscall.ByteSliceFromString
 -512.01kB  0.93% 13.93%  -512.07kB  0.93%  encoding/json.(*decodeState).literalStore
 -492.02kB   0.9% 14.83%  -492.02kB   0.9%  reflect.growslice
    0.05kB 8.5e-05% 14.83%   512.41kB  0.93%  github.com/stretchr/testify/assert.CallerInfo

In the file above we can see that several of the json.Unmarshal calls are removed.

@leoromanovsky leoromanovsky merged commit 9f836c7 into main Feb 15, 2024
2 of 3 checks passed
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.

None yet

1 participant