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

Documenting performance model of empty!, sizehint!, push! & friends #31855

Closed
tpapp opened this issue Apr 27, 2019 · 2 comments · Fixed by #37853
Closed

Documenting performance model of empty!, sizehint!, push! & friends #31855

tpapp opened this issue Apr 27, 2019 · 2 comments · Fixed by #37853

Comments

@tpapp
Copy link
Contributor

tpapp commented Apr 27, 2019

(opening an issue as a follow-up to this discussion)

It is a common idiom to allocate some mutable container, push! or append! values into it, optionally by providing a sizehint!, then resize or empty! it, and reuse between runs.

This leads to performant code with little effort, especially if the number of elements collected cannot be known before each run, but are somewhat similar.

However, I learned most of this from discussions like this, not the documentation.

I propose that the following is documented:

  1. push! and append! methods may (but are not required to) preallocate extra storage. They do preallocate for types in Base, using a heuristic which is optimal for the general use case.

  2. sizehint! may control this preallocation. It does this for types in Base.

  3. empty! is nearly costless (and O(1)) for types that support this kind of preallocation.

None of this is breaking, just documenting the status quo.

Furthermore, we could think about an API to query the current sizehint! setting. For example, imagine that one is collecting random elements, usually less than 10, but very infrequently around 10000. empty! would keep that large buffer around, while one could imagine that the caller could check for this and resize to something smaller.

Cf #24909, as for Vector this documentation would then describe an ideal performance model once that is fixed.

@StefanKarpinski
Copy link
Sponsor Member

@JeffBezanson, would you be able to give some feedback here?

@tlienart
Copy link
Sponsor Contributor

As this is an opportunity for improving the docs of empty!, there is an odd trailing part in the docstring: (I'm on 1.3.0-dev251)

empty!(cb)

Reset the buffer.

which should either be removed or clarified I'd think (happy to open another issue but it seems an easy fix that could piggyback on this)

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 a pull request may close this issue.

3 participants