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

gen4: implement a growable semantics.TableSet #8905

Merged
merged 2 commits into from
Sep 30, 2021
Merged

Conversation

vmg
Copy link
Collaborator

@vmg vmg commented Sep 29, 2021

Description

This was requested by @systay on Slack. We need a TableSet data structure that can support more than 64 tables. We're doing this right here in the smallest possible space: TableSet is now 16 bytes large and packs a small, single-word bitmap that is used when the set contains less than 64 tables, and a pointer to an implementation of an arbitrarily large set.

This implementation is pretty straightforward -- I don't believe it can be optimized further like we'd do in C/C++/Rust. For completeness, these kind of implementations are usually done like this in non-GC'ed languages:

Instead of the 16-byte struct we have here with struct TableSet{ small uint64; large *largeTableSet }, we'd have a single-field struct with a single word in it: struct TableSet { set uintptr }. When trying to read or write to this set, we'd check if ts.set & 0x1 != 0; if this is true, then we have a 63 bit tableset by doing ts.set >> 1. If it's false, then we'd have a large tableset by doing (*largeTableSet)(ts.set). This works in practice because memory allocators will never return pointers with a 1-off alignment (their minimal alignment is at the very least the system's word size, 4/8), so we can store a bitmap in the pointer as long as the last bit of the bitmap is always 1. 👌

...Of course this doesn't work in a garbage collected language like Go because if you leave a *largeTableSet pointed at by an uintptr, the GC won't find it and it'll be freed on a later GC pass. So we don't get to to pack pointers and we need to run with 16-byte TableSets.

Good enough!

cc @systay @frouioui

Related Issue(s)

#7280

Checklist

  • Tests were added or are not required
  • Documentation was added or is not required

Copy link
Member

@frouioui frouioui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The following errors appear when compiling:

abstract/concatenate.go:41:12: invalid operation: tableSet |= source.TableID() (operator | not defined on struct)
abstract/querygraph.go:81:29: invalid operation: lhs | rhs (operator | not defined on struct)

go/vt/vtgate/semantics/tabletset.go Show resolved Hide resolved
Copy link
Collaborator

@systay systay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love it! Thank you thank you thank you

@systay systay changed the title semantics: implement a growable tableset gen4: implement a growable semantics.TableSet Sep 29, 2021
@systay systay added the Benchmark me Add label to PR to run benchmarks label Sep 29, 2021
@vmg vmg merged commit b6c3566 into vitessio:main Sep 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants