sergot / http-useragent

Web user agent class for Perl 6.
MIT License
36 stars 39 forks source link

Pre-allocate a buffer when getting content #187

Closed MasterDuke17 closed 6 years ago

MasterDuke17 commented 6 years ago

Instead of growing it with ~=. This reduced the amount of memory used considerably.

MasterDuke17 commented 6 years ago

maxrss is reduced to 219.3125MiB when running https://gist.github.com/AlexDaniel/0cdb3211cfcae90c29da9330649171c2 (down from 1500MiB).

MasterDuke17 commented 6 years ago

Turns out I've mixed up two different changes and I need to get them sorted out before this can be merged.

MasterDuke17 commented 6 years ago

@sergot @ugexe @jonathanstowe any idea what's up with the failing test? It's saying Could not connect socket: Connection refused, so it looks like it might be a problem in https://github.com/sergot/http-useragent/blob/master/t/230-binary-request.t#L40 (and not necessarily my code), but the test passes locally for me.

AlexDaniel commented 6 years ago

Seems to be ok now?

sergot commented 6 years ago

Restarted the build and it passed. :+1:

AlexDaniel commented 6 years ago

Merge it then? :)

AlexDaniel commented 6 years ago

OK, I hate to say it, but can we revert this please? :)

This is currently making Pastebin::Gist uninstallable because it hangs with 100% CPU on its tests. I've noticed this issue indirectly in whateverable also. Note that HTTP::UserAgent tests are not affected by this regression (with and without NETWORK_TESTING), which means that this should be resolved with tests. Until then a revert will do.

AlexDaniel commented 6 years ago

See this PR instead of reverting: https://github.com/sergot/http-useragent/pull/188