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

[feature/mem-optimizations] Optimize memory usage #121

Merged
merged 6 commits into from
Jan 23, 2025

Conversation

felix-schwarz
Copy link
Collaborator

@felix-schwarz felix-schwarz commented Oct 1, 2024

Description

This PR reduces the memory consumption of the SDK.

Opportunities

  • save avatar data (which can grow large quickly) as separate files rather than as part of OCBookmarks which are kept in memory
  • thin out class setting metadata in low memory configuration

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

@CLAassistant
Copy link

CLAassistant commented Oct 1, 2024

CLA assistant check
All committers have signed the CLA.

@felix-schwarz felix-schwarz requested a review from hosy January 22, 2025 09:35
- OCBookmark:
	- add `metaDataStorageURL` property for transitioning data from internal/memory to external/disk storage
	- if `metaDataStorageURL` is set, externalize avatar/avatarData and load it only on demand for as short as possible
	- keep track of whether an avatar has been stored via avatarDataURL
- OCBookmarkManager
	- add code to transition bookmarks from internal to external storage
	- add code to store changed and transitioned data to external storage
- effectively disable caching in minimum memory configuration for OCCore's thumbnail cache and OCResourceManager's OCResource cache
…ed - and instead implement what was intended: prevent caching altogether
@felix-schwarz felix-schwarz force-pushed the feature/mem-optimizations branch from a6aff7a to 7bfe325 Compare January 22, 2025 09:48
@felix-schwarz felix-schwarz merged commit 5b3598b into master Jan 23, 2025
2 checks passed
@felix-schwarz
Copy link
Collaborator Author

@hosy I had to merge this PR in order to resolve merge conflicts in owncloud/ios-app#1430 (which required merging with ios-app/master, which - already having merged the ios-app portion of this PR - in turn requires the SDK changes in this PR). The one finding that turned up during CR was addressed - if there's more to address, please let me know.

felix-schwarz added a commit that referenced this pull request Jan 30, 2025
… to avoid libSwiftCore.dylib overhead/permanent memory allocation, saving up to several MBs (addition to #121)

- Localizable.xcstrings: mark stale strings as manual
@felix-schwarz
Copy link
Collaborator Author

@hosy FWIW I made a small additional change directly in master in 4f570ca which has a surprisingly huge effect on memory usage (several MBs!)

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