Skip to content

funk: clean up join API #4853

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

Merged
merged 1 commit into from
Apr 30, 2025
Merged

funk: clean up join API #4853

merged 1 commit into from
Apr 30, 2025

Conversation

ripatel-fd
Copy link
Contributor

@ripatel-fd ripatel-fd commented Apr 23, 2025

Adds a 'join' struct containing local address space pointers to
sub objects. Improves insert performance by about 20ns per record.

Reverts 'ele_max' to ulong to avoid integer truncation issues.

Closes #4847

@ripatel-fd ripatel-fd changed the title ripatel/funk join funk: clean up join API Apr 23, 2025
@ripatel-fd ripatel-fd marked this pull request as ready for review April 23, 2025 17:52
@ripatel-fd ripatel-fd force-pushed the ripatel/funk-join branch 7 times, most recently from b4b0b30 to 760d2f2 Compare April 23, 2025 20:06
@asiegel-jt
Copy link
Contributor

I'm not sure about the value in this change. Accessing a relative pointer is about as fast as an absolute pointer, and the compiler should be optimizing this case correctly. You're making a very large change for a relatively trivial gain.

@asiegel-jt
Copy link
Contributor

The spirit behind this change makes sense from an aesthetic point of view though. It does make the API a bit less confusing. You need to fix the unit tests however.

@ripatel-fd
Copy link
Contributor Author

I'm not sure about the value in this change. Accessing a relative pointer is about as fast as an absolute pointer, and the compiler should be optimizing this case correctly. You're making a very large change for a relatively trivial gain.

@asiegel-jt This change yields a significant performance improvement (about 20ns per record written to funk). The problem was that all these join external function calls stall the CPU's pipeline quite often. The join struct eliminates those join calls.

@jumpsiegel
Copy link
Contributor

I think alex said he would approve once the tests pass

@ripatel-fd ripatel-fd force-pushed the ripatel/funk-join branch 4 times, most recently from 66b1137 to 1e65dfa Compare April 30, 2025 20:33
Adds a 'join' struct containing local address space pointers to
sub objects.  Improves insert performance by about 20ns per record.

Reverts 'ele_max' to ulong to avoid integer truncation issues.
@ripatel-fd ripatel-fd added this pull request to the merge queue Apr 30, 2025
Merged via the queue into main with commit 4eba2c6 Apr 30, 2025
9 checks passed
@ripatel-fd ripatel-fd deleted the ripatel/funk-join branch April 30, 2025 21:06
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.

[Snapshot Performance] Improve funk join pattern
5 participants