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

RUMM-610 Add memory consumption benchmarks for data upload #181

Merged
merged 2 commits into from
Jul 15, 2020

Conversation

ncreated
Copy link
Member

What and why?

📦 This PR adds benchmarks required to test memory leaks fixed in #180. I'm adding this in a separate PR to master, as on the hotfix branch (starting from 1.2.2), the benchmark target's setup is incomplete.

How?

I added HTTPServerMock instrumentation to the Benchmarks target (to run them against real server).

The test is simple:

measure(metrics: [XCTMemoryMetric()]) {
    (0..<10).forEach { _ in
        let data = Data(repeating: 0x41, count: 10 * 1_024 * 1_024)
        _ = dataUploader.upload(data: data)
    }
}

It measures the memory footprint of uploading 10 data requests, 10MB each.

Running without the fix (we can clearly see over 100MBs of leaked data):

[Memory Physical, kB] 
average: 104956.723, 
relative standard deviation: 0.049%, 
values: [104943.616000, 104910.848000, 104968.192000, 105050.112000, 104910.848000]

Running with #180 hotfix (the memory footprint is negligible):

[Memory Physical, kB] 
average: -2048.819, 
relative standard deviation: -148.466%, 
values: [-7790.592000, -2560.000000, 28.672000, 36.864000, 40.960000],

Review checklist

  • Feature or bugfix MUST have appropriate tests (unit, integration)
  • Make sure each commit and the PR mention the Issue number or JIRA reference

@ncreated ncreated requested a review from a team as a code owner July 14, 2020 21:20
@ncreated ncreated self-assigned this Jul 14, 2020
@ncreated ncreated merged commit c9d56a6 into master Jul 15, 2020
@ncreated ncreated deleted the ncreated/RUMM-610-add-memory-consumption-benchmarks branch July 15, 2020 11:41
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.

3 participants