tisonkun commented on PR #144: URL: https://github.com/apache/datasketches-rust/pull/144#issuecomment-4879974141
@k3dom > active_entries() could probably be a lazy iterator impl Iterator<Item = (&T, u64)> + '_ instead of collecting Vec<(&T, u64)> which would make it totally allocation-free, not sure if it would be always faster though for very large maps. This is proposed by my Codex at the first time. But I reject it because an `active_entries` iterator would do the activity check every time while we'd iterator it several (now 3) times. So it's not a net win. > Would probably also make sense to update frequent_items_with_threshold as a follow-up because here we still clone every qualifying item into owned Row's before the caller can filter This may be possible. But now the returned rows are sorted so it seems not to be suitable to return an iterator. If you mean we may change `Row<T>` to `Row<'a, T>`, as it's used by end users, I'm afraid that would increase usage complexity - we may review this when we have a concrete use case that proves its benefits. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
