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

Alloc proc read buffer based on amount last read #631

Merged
merged 4 commits into from Sep 8, 2017

Conversation

MasterDuke17
Copy link
Contributor

Instead of using the suggested_size (which is almost always 65536
according to http://docs.libuv.org/en/v1.x/handle.html#data-types), use
the next power of two greater than the amount last read (which has been
added to the SpawnInfo struct).

Passes NQP's make m-test and Rakudo's make m-test m-spectest.

For https://gist.github.com/MasterDuke17/685b627a6a2749483dc5ec09c6a777a4
with only 5 iterations, compared to HEAD, heaptrack reports 654m "bytes allocated in total"
instead of 1.1g, 255m peak memory consumption instead of 387m, and on_alloc in
"most memory allocated" using 4.4m instead of 480m.

Instead of using the suggested_size (which is almost always 65536
according to http://docs.libuv.org/en/v1.x/handle.html#data-types), use
the next power of two greater than the amount last read (which has been
added to the SpawnInfo struct).
Also added comment about equivalent code and why we're doing
bit-twiddling.
@ugexe
Copy link
Contributor

ugexe commented Aug 12, 2017

Wouldn't this be obsoleted by further implementing $*DEFAULT-READ-ELEMS?

@jnthn
Copy link
Member

jnthn commented Sep 8, 2017

Even if $*DEFAULT-READ-ELEMS were made to affect this some day, it'd still not provide automatic tuning as we achieve here. So I don't think it's either-or. (I'm not quite sure if $*DEFAULT-READ-ELEMS should affect this or not.)

@jnthn jnthn merged commit e70f897 into MoarVM:master Sep 8, 2017
@MasterDuke17 MasterDuke17 deleted the smarter_on_alloc branch September 8, 2017 21:32
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

3 participants