fix: reduce memory and cpu usage while running background queue #379

levibostian · 2023-09-06T15:43:39Z

part of: https://github.com/customerio/issues/issues/10950

While profiling the SDK, I noticed that the QueueRunner was using a lot of resources while the BQ executed. I found that for every background task item that was executed, a lot of new dependencies were being created because a new QueueRunner instance (and it's subdepenencies) were being constructed by the hooks manager.

I tried a couple of solutions to prevent new QueueRunner instances being created for each BQ task execution, but each solution I came up with would introduce a new edge case to the BQ execution where tasks might not get picked up by a QueueRunner instance with hooks.

Instead, I added commits later on in this branch that introduced the solution of no longer using hooks for executing the background queue. I don't see a reason to use hooks for BQ execution in the long-term and because it's what is causing the issues fixed by this PR, I removed them.

This reverts commit 671b636.

github-actions · 2023-09-06T15:43:51Z

Pull request title looks good 👍!

If this pull request gets merged, it will cause a new release of the software. Example: If this project's latest release version is 1.0.0. If this pull request gets merged in, the next release of this project will be 1.0.1. This pull request is not a breaking change.

All merged pull requests will eventually get deployed. But some types of pull requests will trigger a deployment (such as features and bug fixes) while some pull requests will wait to get deployed until a later time.

This project uses a special format for pull requests titles. Expand this section to learn more (expand by clicking the ᐅ symbol on the left side of this sentence)...

This project uses a special format for pull requests titles. Don't worry, it's easy!

This pull request title should be in this format:

<type>: short description of change being made

If your pull request introduces breaking changes to the code, use this format:

<type>!: short description of breaking change

where <type> is one of the following:

feat: - A feature is being added or modified by this pull request. Use this if you made any changes to any of the features of the project.
fix: - A bug is being fixed by this pull request. Use this if you made any fixes to bugs in the project.
docs: - This pull request is making documentation changes, only.
refactor: - A change was made that doesn't fix a bug or add a feature.
test: - Adds missing tests or fixes broken tests.
style: - Changes that do not effect the code (whitespace, linting, formatting, semi-colons, etc)
perf: - Changes improve performance of the code.
build: - Changes to the build system (maven, npm, gulp, etc)
ci: - Changes to the CI build system (Travis, GitHub Actions, Circle, etc)
chore: - Other changes to project that don't modify source code or test files.
revert: - Reverts a previous commit that was made.

Examples:

feat: edit profile photo
refactor!: remove deprecated v1 endpoints
build: update npm dependencies
style: run formatter

Need more examples? Want to learn more about this format? Check out the official docs.

Note: If your pull request does multiple things such as adding a feature and makes changes to the CI server and fixes some bugs then you might want to consider splitting this pull request up into multiple smaller pull requests.

github-actions · 2023-09-06T15:43:55Z

Sample app builds 📱

Below you will find the list of the latest versions of the sample apps. It's recommended to always download the latest builds of the sample apps to accurately test the pull request.

APN-UIKit: levi/reduce-memory-usage (1694039406)
CocoaPods-FCM: levi/reduce-memory-usage (1694039439)

Shahroz16

Good catch!

…dge case

Sources/Common/Background Queue/QueueRunner.swift

levibostian · 2023-09-06T22:32:37Z

PR implementation changed so I asked for a re-review.

I updated the PR description talking about it.

levibostian · 2023-09-06T22:33:39Z

Sources/Common/Background Queue/QueueRunner.swift

@@ -13,20 +13,13 @@ public protocol QueueRunner: AutoMockable {



This file contains the most changes for the PR. I moved code from Tracking module's QueueRunner into this one. Now, all of the Tracking API HTTP calls are in 1 module (which, I think is a good thing thinking about modularity in the code base).

levibostian · 2023-09-06T22:34:29Z

Sources/Common/Background Queue/Task Data/DeletePushNotificationQueueTaskData.swift

@@ -0,0 +1,16 @@
+import Foundation
+
+public struct DeletePushNotificationQueueTaskData: Codable {


Lots of files in this PR are structs that git says are new files.

They are not new, they are moved and I modified them from internal to public so code in tracking module could access them.

levibostian · 2023-09-06T22:34:55Z

Sources/Common/Background Queue/Type/QueueTaskType.swift

+    case identifyProfile
+    case trackEvent
+    case registerPushToken
+    case deletePushToken
+    case trackPushMetric


Part of moving code from Tracking QueueRunner into 1 QueueRunner.

Shahroz16

This is what I understand. Please correct me If I am wrong and ask for a re-review.

Before we had two Queue Runners, one in common and one in tracking. We utilized hooks to communicate between modules (common and tracking).

Now with this change, we are getting rid of the Queue runner in the tracking module and shifting all the logic from there to the common queue logic also moved the data models from tracking to common so they can be utilized in the updated Queue runner in common.

So there is no new code that is being added but rather code being shifted from one module to another and we are getting rid of a particular hook that was used for this purpose.

levibostian · 2023-09-07T15:04:46Z

@Shahroz16, 100% correct. I couldn't have put it better myself.

Communication between modules is valuable and does help us keep our SDKs lightweight and modular. There is value in it. However, I no longer believe that a QueueRunner is the best use of the hooks feature. Originally, separate QueueRunners were to try and keep the SDK extra slim by putting specific Track API calls into modules that use them. But, doing that has added more complexity then what it's worth.

Also, from internal discussions we have had about splitting our Common module into more modules, we have mentioned having a module that contains all HTTP code for communicating with the Track API. So, I see a day where all CIO Track API code is in 1 module. Therefore, another reason to not spread Track API code across modules.

By this PR, we receive a performance gain in the SDK while also putting all Track API code into 1 module, which we can then split into it's own module in the future.

## [2.8.2](2.8.1...2.8.2) (2023-09-07) ### Bug Fixes * reduce memory and cpu usage while running background queue ([#379](#379)) ([87a7eed](87a7eed))

levibostian added 3 commits August 31, 2023 14:28

fix callback issue

671b636

fix: reduce memory and cpu usage while running background queue

4266ecb

Revert "fix callback issue"

f6a36c8

This reverts commit 671b636.

levibostian added 2 commits September 6, 2023 11:00

create test

ea8d1da

Merge branch 'main' into levi/reduce-memory-usage

1fa7e53

levibostian marked this pull request as ready for review September 6, 2023 16:01

levibostian requested a review from a team September 6, 2023 16:01

Shahroz16 approved these changes Sep 6, 2023

View reviewed changes

reset the re-used queue hooks before each run to prevent a possible e…

052c005

…dge case

levibostian requested review from Shahroz16 and a team September 6, 2023 19:36

levibostian marked this pull request as draft September 6, 2023 19:38

remove queue runner hooks

cee5ae0

levibostian commented Sep 6, 2023

View reviewed changes

Sources/Common/Background Queue/QueueRunner.swift Outdated Show resolved Hide resolved

Apply suggestions from code review

5cd94b5

levibostian marked this pull request as ready for review September 6, 2023 22:28

fix lint

4f49b85

levibostian commented Sep 6, 2023

View reviewed changes

Shahroz16 approved these changes Sep 7, 2023

View reviewed changes

levibostian merged commit 87a7eed into main Sep 7, 2023
9 checks passed

levibostian deleted the levi/reduce-memory-usage branch September 7, 2023 15:23

github-actions bot pushed a commit that referenced this pull request Sep 7, 2023

chore: prepare for 2.8.2

e150a62

## [2.8.2](2.8.1...2.8.2) (2023-09-07) ### Bug Fixes * reduce memory and cpu usage while running background queue ([#379](#379)) ([87a7eed](87a7eed))

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: reduce memory and cpu usage while running background queue #379

fix: reduce memory and cpu usage while running background queue #379

levibostian commented Sep 6, 2023 •

edited

github-actions bot commented Sep 6, 2023 •

edited

Examples:

github-actions bot commented Sep 6, 2023 •

edited

Shahroz16 left a comment

levibostian commented Sep 6, 2023

levibostian Sep 6, 2023

levibostian Sep 6, 2023

levibostian Sep 6, 2023

Shahroz16 left a comment

levibostian commented Sep 7, 2023 •

edited

		@@ -13,20 +13,13 @@ public protocol QueueRunner: AutoMockable {

		@@ -0,0 +1,16 @@
		import Foundation

		public struct DeletePushNotificationQueueTaskData: Codable {

fix: reduce memory and cpu usage while running background queue #379

fix: reduce memory and cpu usage while running background queue #379

Conversation

levibostian commented Sep 6, 2023 • edited

github-actions bot commented Sep 6, 2023 • edited

Examples:

github-actions bot commented Sep 6, 2023 • edited

Sample app builds 📱

Shahroz16 left a comment

Choose a reason for hiding this comment

levibostian commented Sep 6, 2023

levibostian Sep 6, 2023

Choose a reason for hiding this comment

levibostian Sep 6, 2023

Choose a reason for hiding this comment

levibostian Sep 6, 2023

Choose a reason for hiding this comment

Shahroz16 left a comment

Choose a reason for hiding this comment

levibostian commented Sep 7, 2023 • edited

levibostian commented Sep 6, 2023 •

edited

github-actions bot commented Sep 6, 2023 •

edited

github-actions bot commented Sep 6, 2023 •

edited

levibostian commented Sep 7, 2023 •

edited