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

Improve memory handling #41

Merged
merged 5 commits into from
May 23, 2016
Merged

Improve memory handling #41

merged 5 commits into from
May 23, 2016

Conversation

zenangst
Copy link
Contributor

@zenangst zenangst commented May 23, 2016

This PR aims to improve the memory handling when using Cache.

Before, we shared the same variable for DiskStorage and MemoryStorage which ended up not being what we want. And here is why:

DiskStorage’s limit is based on actual disk space where as MemoryStorage is based on the amount of objects that you want to keep in memory.

So this PR adds another variable to the configuration called maxObjects, it defaults to 0 so that it does not break the current implementations but the components using Cache now gain more fine-grained control over both storages.

Also, internally inside didSet in MemoryStorage but it was never called because it was only called inside init.

As and added bonus, this PR also adds SwiftLint and fixes some of the warnings that it introduces.

With Imaginary this change made quiet an impact on memory usage;

Before

screen shot 2016-05-23 at 11 01 49

### After

screen shot 2016-05-23 at 11 04 26

@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @vadymmarkov and @attila-at-hyper to be potential reviewers

@@ -37,6 +38,9 @@ public struct DefaultCacheConverter<T> {

/**
Encodes an instance to NSData

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sometimes I feel the documentation is a bit unnecessary. I can see that by looking at the function signature

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, they are a bit obvious but where would you draw the line? Also, the linter forces you to add documentation.

@onmyway133 onmyway133 merged commit 36ef824 into master May 23, 2016
@onmyway133 onmyway133 deleted the improve/memory-handling branch May 23, 2016 09:22
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