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.

Sign up for GitHub

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

Jump to bottom

cache table list #8127

Merged
merged 7 commits into from
Jul 17, 2024
Merged

cache table list #8127

merged 7 commits into from
Jul 17, 2024

Conversation

max-hoffman
Copy link
Contributor

@max-hoffman max-hoffman commented Jul 16, 2024 •
edited
Loading

RootValue caches a hash of its table list, which is invalidated when we update the table address list. The hash is used to cache a map of table names in the root value and expedites table retrieval during planbuilder. The cache could be used elsewhere for performance in the future.

dolthub/doltgresql#503

@coffeegoddd
Copy link
Contributor

@max-hoffman DOLT

comparing_percentages
100.000000 to 100.000000
version result total
8d66bca ok 5937457
version total_tests
8d66bca 5937457
correctness_percentage
100.0

@max-hoffman
Copy link
Contributor Author

#benchmark

@github-actions GitHub Actions
Copy link

@max-hoffman workflow run: https://github.com/dolthub/dolt/actions/runs/9965921438

@coffeegoddd
Copy link
Contributor

@max-hoffman DOLT

comparing_percentages
100.000000 to 100.000000
version result total
cda8e62 ok 5937457
version total_tests
cda8e62 5937457
correctness_percentage
100.0

@coffeegoddd
Copy link
Contributor

@max-hoffman DOLT

test_name from_latency_p95 to_latency_p95 is_faster
tpcc-scale-factor-1 78.6 70.55 0
test_name server_name server_version tps test_name server_name server_version tps is_faster
tpcc-scale-factor-1 dolt 8c43b2e 32.06 tpcc-scale-factor-1 dolt cda8e62 34.14 0

@coffeegoddd
Copy link
Contributor

@coffeegoddd DOLT

comparing_percentages
100.000000 to 100.000000
version result total
9d80114 ok 5937457
version total_tests
9d80114 5937457
correctness_percentage
100.0

@coffeegoddd
Copy link
Contributor

@max-hoffman DOLT

read_tests from_latency_median to_latency_median is_faster
covering_index_scan 3.07 2.97 0
groupby_scan 17.63 17.32 0
index_join 5.28 5.37 0
index_join_scan 2.61 2.71 0
index_scan 54.83 54.83 0
oltp_point_select 0.47 0.46 0
oltp_read_only 7.84 7.7 0
select_random_points 0.77 0.75 0
select_random_ranges 0.92 0.89 0
table_scan 56.84 56.84 0
types_table_scan 144.97 144.97 0
write_tests from_latency_median to_latency_median is_faster
oltp_delete_insert 6.21 6.09 0
oltp_insert 3.07 3.02 0
oltp_read_write 14.21 13.95 0
oltp_update_index 3.13 3.07 0
oltp_update_non_index 3.07 3.02 0
oltp_write_only 6.43 6.43 0
types_delete_insert 6.79 6.67 0

@max-hoffman
Copy link
Contributor Author

#benchmark

@github-actions GitHub Actions
Copy link

@max-hoffman workflow run: https://github.com/dolthub/dolt/actions/runs/9978166494

@max-hoffman
Copy link
Contributor Author

#benchmark

@github-actions GitHub Actions
Copy link

@max-hoffman workflow run: https://github.com/dolthub/dolt/actions/runs/9978242251

@coffeegoddd
Copy link
Contributor

@max-hoffman DOLT

comparing_percentages
100.000000 to 100.000000
version result total
fd3b3b1 ok 5937457
version total_tests
fd3b3b1 5937457
correctness_percentage
100.0

@coffeegoddd
Copy link
Contributor

@max-hoffman DOLT

test_name from_latency_p95 to_latency_p95 is_faster
tpcc-scale-factor-1 74.46 71.83 0
test_name server_name server_version tps test_name server_name server_version tps is_faster
tpcc-scale-factor-1 dolt 8c43b2e 32.83 tpcc-scale-factor-1 dolt 7294424 34.16 0

@coffeegoddd
Copy link
Contributor

@max-hoffman DOLT

test_name from_latency_p95 to_latency_p95 is_faster
tpcc-scale-factor-1 74.46 71.83 0
test_name server_name server_version tps test_name server_name server_version tps is_faster
tpcc-scale-factor-1 dolt 8c43b2e 32.24 tpcc-scale-factor-1 dolt fd3b3b1 34.17 0

reltuk
reltuk approved these changes Jul 17, 2024
View reviewed changes
Copy link
Contributor

@reltuk reltuk left a comment

Choose a reason for hiding this comment

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

LGTM!

go/libraries/doltcore/doltdb/root_val.go Outdated
@@ -569,14 +578,21 @@ func (root *rootValue) GetTableNames(ctx context.Context, schemaName string) ([]
return nil, err
}

md5 := xxhash.New()
Copy link
Contributor

Choose a reason for hiding this comment

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

confusing name for a hasher that isn't computing an md5.

@coffeegoddd
Copy link
Contributor

@max-hoffman DOLT

comparing_percentages
100.000000 to 100.000000
version result total
f5c8de1 ok 5937457
version total_tests
f5c8de1 5937457
correctness_percentage
100.0

@coffeegoddd
Copy link
Contributor

@max-hoffman DOLT

read_tests from_latency_median to_latency_median is_faster
covering_index_scan 3.07 2.97 0
groupby_scan 17.32 17.32 0
index_join 5.47 5.37 0
index_join_scan 2.61 2.57 0
index_scan 55.82 55.82 0
oltp_point_select 0.47 0.46 0
oltp_read_only 7.84 7.7 0
select_random_points 0.77 0.75 0
select_random_ranges 0.92 0.89 0
table_scan 56.84 56.84 0
types_table_scan 144.97 147.61 0
write_tests from_latency_median to_latency_median is_faster
oltp_delete_insert 6.09 6.09 0
oltp_insert 3.02 3.02 0
oltp_read_write 14.21 13.95 0
oltp_update_index 3.07 3.07 0
oltp_update_non_index 3.02 3.02 0
oltp_write_only 6.43 6.43 0
types_delete_insert 6.67 6.67 0

@coffeegoddd
Copy link
Contributor

@max-hoffman DOLT

read_tests from_latency_median to_latency_median is_faster
covering_index_scan 3.02 3.02 0
groupby_scan 17.32 17.32 0
index_join 5.37 5.47 0
index_join_scan 2.57 2.57 0
index_scan 54.83 54.83 0
oltp_point_select 0.46 0.46 0
oltp_read_only 7.84 7.56 0
select_random_points 0.75 0.74 0
select_random_ranges 0.9 0.89 0
table_scan 55.82 55.82 0
types_table_scan 144.97 144.97 0
write_tests from_latency_median to_latency_median is_faster
oltp_delete_insert 6.09 6.09 0
oltp_insert 3.02 3.02 0
oltp_read_write 14.21 13.95 0
oltp_update_index 3.07 3.07 0
oltp_update_non_index 3.02 3.02 0
oltp_write_only 6.43 6.32 0
types_delete_insert 6.67 6.67 0

@max-hoffman max-hoffman mentioned this pull request Jul 17, 2024
Merged
@max-hoffman max-hoffman merged commit 654dd37 into main Jul 17, 2024
19 of 21 checks passed
@max-hoffman max-hoffman deleted the max/cache-table-list branch July 17, 2024 18:27
@BrewTestBot BrewTestBot mentioned this pull request Jul 17, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@reltuk reltuk reltuk approved these changes

Assignees
No one assigned
Labels
correctness_approved
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@max-hoffman @coffeegoddd @reltuk