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

ZipOuterJoin should have a fast path for single chunks #1040

Open
ritchie46 opened this issue Jul 25, 2021 · 8 comments
Open

ZipOuterJoin should have a fast path for single chunks #1040

ritchie46 opened this issue Jul 25, 2021 · 8 comments

Comments

@ritchie46
Copy link
Member

Currently the TakeRandom traits are used, but this is expensive for the most common case, single chunks.

@marcvanheerden
Copy link
Contributor

I'll pick this up if thats okay

@ritchie46
Copy link
Member Author

Ah, yes! 🙂

@marcvanheerden
Copy link
Contributor

Hi, @ritchie46 I attempted this by using .get_unchecked() directly on the chunked array when there is only one chunk.

marcvanheerden@71a1b34

This didn't improve the performance noticeably. I tested it out on an outer join of two 10mil row tables. Have I misunderstood the request?

@ritchie46
Copy link
Member Author

The idea was to unpack to the underlaying PrimitiveArray instead of using the take_rand proxy.

We can get the PrimitiveArray like this: self.downcast_iter().next().unwrap();

@marcvanheerden
Copy link
Contributor

I tried it in this commit on my fork: marcvanheerden@edcf5cd

Still not getting a measurable run time difference on the same two 10mil row tables. I used hyperfine for the measurement and the change is slower by 10ms on around 7s for the join. I think this is just noise and not an actual regression.

Let me know if I'm doing something wrong

@ritchie46
Copy link
Member Author

@marcvanheerden I've left some comments that should help the performance.

@marcvanheerden
Copy link
Contributor

Thanks for your help again, I've made the change here. Unforunately I'm not seeing a performance improvement.

If I'm reading these flamegraphs correctly it looks like:

  1. The changes don't make a big difference
  2. The piece we're working on is quite a small portion of the runtime overall so there isn't much time to be gained.

Here are the before and after flamegraphs.
Archive.zip

Let me know if I'm making a mistake or missing something.

Thanks

@ritchie46
Copy link
Member Author

An outer join does also quite some work. Maybe should isolate the functionality we're benching?

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

No branches or pull requests

2 participants