Skip to content

Conversation

@jacoscaz
Copy link
Collaborator

@jacoscaz jacoscaz commented Mar 30, 2022

This PR contains all work from #48 , #49 , #53, #54 and #55 done by @jeswr and I on performance. It's not ready to be merged but it is ready for reviews and feedbacks.

Jesse and I opted to merge everything into one branch as, having validated all of these PRs individually, we now need them in aggregate to measure their overall effect and to facilitate testing within other projects, including Jesse's performance evaluations for his honors work.

As for the state of this PR, all asynciterator tests are passing and so are Comunica's spec:query and spec:update suites. As per #49 (comment) , these are the issues within Comunica:

@coveralls
Copy link

coveralls commented Mar 30, 2022

Coverage Status

Coverage decreased (-1.5%) to 98.452% when pulling 3789177 on jacoscaz:faster-filtering-mapping-limiting-skipping-wrapping-union into 358608f on RubenVerborgh:main.

@RubenVerborgh
Copy link
Owner

Thanks @jacoscaz and @jeswr!

I'm not sure one big PR is the best thing though if you still want me to understand everything 😅
But perhaps it's indeed good to iterate on this until it is in perfect shape, and then we can break this into bits as needed.

@jacoscaz
Copy link
Collaborator Author

But perhaps it's indeed good to iterate on this until it is in perfect shape, and then we can break this into bits as needed.

Yep, we can always break it down into separate PRs later on, if needed.

introduce maybe map and do some refactoring
@jacoscaz jacoscaz changed the title Faster filtering, mapping, limiting, skipping, wrapping and union Faster filtering, mapping, limiting, skipping and wrapping Apr 3, 2022
@jeswr jeswr mentioned this pull request Apr 5, 2022
@jacoscaz
Copy link
Collaborator Author

jacoscaz commented Apr 7, 2022

Closing this in favor of #59 and the plan detailed in #44 (comment) .

@jacoscaz jacoscaz closed this Apr 7, 2022
@jacoscaz jacoscaz deleted the faster-filtering-mapping-limiting-skipping-wrapping-union branch May 9, 2022 08:51
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.

4 participants