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

Optimize some Sequence methods on Range and List #901

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ChayimFriedman2
Copy link
Contributor

@ChayimFriedman2 ChayimFriedman2 commented Dec 31, 2020

The methods that got optimized:

  • toList in both Range and List. They both preallocate enough space instead of keeping adding elements. On List we also use memcpy() to effectively copy the list.
  • count and contains(_) on Range. They both calculates the value now instead of iterating over the range to determine it.
  • take(_) and skip(_) on Range. They both return a new precalculated range instead of a dedicated sequence (TakeSequence and SkipSequence, respectively, in wren_core.wren). This is not a major gain, but it allows the previous optimized method to be used on the result instead of their slower Sequence counterparts.

All methods were implemented in C rather than Wren.

The methods that got optimized:
 - `toList` in both `Range` and `List`. They both preallocate enough space instead of keeping adding elements. On `List` we also use `memcpy()` to effectively copy the list.
 - `count` and `contains(_)` on `Range`. They both calculates the value now instead of iterating over the range to determine it.
 - `take(_)` and `skip(_)` on `Range`. They both return a new precalculated range instead of a dedicated sequence (`TakeSequence` and `SkipSequence`, respectively, in wren_core.wren). This is not a major gain, but it allows the previous optimized method to be used on the result instead of their slower `Sequence` counterparts.

All methods were implemented in C rather than Wren.
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.

1 participant