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

ovsdb2ddlog: Add --intern-table CLI switch. #934

Merged
merged 1 commit into from Mar 11, 2021

Conversation

ryzhyk
Copy link
Contributor

@ryzhyk ryzhyk commented Mar 10, 2021

Added --intern-table flag to the compiler to declare input tables coming from
OVSDB as Intern<...>. This is useful for tables whose records are copied
around as a whole and can therefore benefit from interning performance- and
memory-wise. In the past we had to create a separate table and copy records
from the original input table to it while wrapping them in Intern<>. With
this change, we avoid the extra copy and intern records as we ingest them
for selected tables.

I am not pushing any tests for the new feature with this commit.
Testing was done using the master branch of OVN that currently has a
bunch of nondet test failures, presumably due to race conditions in the
test harness.

Added `--intern-table` flag to the compiler to declare input tables coming from
OVSDB as `Intern<...>`.  This is useful for tables whose records are copied
around as a whole and can therefore benefit from interning performance- and
memory-wise.  In the past we had to create a separate table and copy records
from the original input table to it while wrapping them in `Intern<>`.  With
this change, we avoid the extra copy and intern records as we ingest them
for selected tables.

I am not pushing any tests for the new feature with this commit.
Testing was done using the master branch of OVN that currently has a
bunch of nondet test failures, presumably due to race conditions in the
test harness.
@ryzhyk ryzhyk requested a review from blp March 10, 2021 23:27
Copy link
Contributor

@blp blp left a comment

Choose a reason for hiding this comment

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

Seems useful. Thank you! I did not review the Haskell.

Do we have a place to document ovsdb2tool? As is, the CHANGELOG adds useful advice on how to use the new option, but changelogs are not the first place I look for documentation.

@ryzhyk
Copy link
Contributor Author

ryzhyk commented Mar 11, 2021

Seems useful. Thank you! I did not review the Haskell.

Do we have a place to document ovsdb2tool? As is, the CHANGELOG adds useful advice on how to use the new option, but changelogs are not the first place I look for documentation.

We don't have any documentation at the moment apart from the usage message printed by the compiler and a detailed comment in the Haskell source.

There should probably be a separate file in the doc folder documenting the compiler.

@ryzhyk ryzhyk merged commit 0e9412d into vmware:master Mar 11, 2021
@ryzhyk ryzhyk deleted the intern-table branch March 11, 2021 18:36
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.

None yet

2 participants