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

Removed DateComponents causing memory leaks #231

Closed
wants to merge 2 commits into from
Closed

Removed DateComponents causing memory leaks #231

wants to merge 2 commits into from

Conversation

lluuaapp
Copy link
Contributor

@tanner0101: as discussed on the Discord channel this helps to the memory usage.

@lluuaapp
Copy link
Contributor Author

Seems the linux-fluent test is failing cause a Swift 5.0 dependency?

/root/fluent-mysql: error: package at '/root/fluent-mysql' requires a minimum Swift tools version of 5.0.0 (currently 4.1.0)

@tanner0101
Copy link
Member

tanner0101 commented Apr 2, 2019

Seems the linux-fluent test is failing cause a Swift 5.0 dependency?

Yes, fixed this on master.

as discussed on the Discord channel this helps to the memory usage.

I think #232 should be sufficient to fix the memory problem for MySQL 3.x. Given this Date stuff isn't a small change, I'd prefer if we could try implementing this in MySQL 4.x instead.

I'm already using gmtime there for the Swift -> MySQL conversion:

https://github.com/vapor/nio-mysql/blob/master/Sources/NIOMySQL/MySQLTime.swift#L53

Would you be interested in helping update the MySQL -> Swift conversion there?

https://github.com/vapor/nio-mysql/blob/master/Sources/NIOMySQL/MySQLTime.swift#L74-L103

If not, I'll leave this open as a reminder to tackle it before the official tag 👍

@lluuaapp
Copy link
Contributor Author

lluuaapp commented Apr 4, 2019

OK, I opened a PR here

@tanner0101 tanner0101 closed this Apr 10, 2019
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

2 participants