[ovs-dev] [PATCH ovn 17/21] ovn-northd-ddlog: Intern selected input relations.
blp at ovn.org
Tue Mar 30 21:38:45 UTC 2021
On Tue, Mar 30, 2021 at 05:52:05PM +0200, Dumitru Ceara wrote:
> On 3/27/21 1:31 AM, Ben Pfaff wrote:
> > From: Leonid Ryzhyk <lryzhyk at vmware.com>
> > DDlog 0.38.0 adds the `--intern-table` CLI flag to the `ovsdb2ddlog`
> > 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 separate tables in `helpers.dl` and copy
> > records from the original input table to them while wrapping them in
> > `Intern<>`. With this change, we avoid the extra copy and intern
> > records as we ingest them for selected tables.
> > We use the `--intern-table` flag to eliminate all intermediate tables in
> > `helpers.dl`.
> > Signed-off-by: Leonid Ryzhyk <lryzhyk at vmware.com>
> > Signed-off-by: Ben Pfaff <blp at ovn.org>
> > ---
> Hi Ben, Leonid,
> This is not a full review, I just noticed that build fails with this
> patch applied.
> > diff --git a/northd/ovsdb2ddlog2c b/northd/ovsdb2ddlog2c
> > index c66ad81073e1..19aeb265b633 100755
> > --- a/northd/ovsdb2ddlog2c
> > +++ b/northd/ovsdb2ddlog2c
> > @@ -36,6 +36,7 @@ The following ovsdb2ddlog options are supported:
> > --output-only-table=TABLE Mark TABLE as output-only. DDlog will send updates to this table directly to OVSDB without comparing it with current OVSDB state.
> > --ro=TABLE.COLUMN Ignored.
> > --rw=TABLE.COLUMN Ignored.
> > + --intern-table=TABLE Ignored.
> > --output-file=FILE.inc Write output to FILE.inc. If this option is not specified, output will be written to stdout.
> > The following options are also available:
> > @@ -52,6 +53,7 @@ if __name__ == "__main__":
> > 'schema-file=',
> > 'output-table=',
> > 'output-only-table=',
> > + 'intern-table=',
> > 'ro=',
> > 'rw=',
> > 'output-file='])
> > @@ -75,9 +77,7 @@ if __name__ == "__main__":
> > schema_file = value
> > elif key in ['-o', '--output-table']:
> > output_tables.add(value)
> > - elif key == '--output-only-table':
> > - output_only_tables.add(value)
> I guess the two lines above were removed by accident because now
> ovsdb2ddlog2c will silently exit when it receives '--output-only-table'
> as argument.
I think that the correct fix here is to not remove those lines. I'll
put that into the next version.
> As a side note, maybe it's good to improve the error reporting too so it
> doesn't silently exit when arguments are not known :)
> > - elif key in ['--ro', '--rw']:
> > + elif key in ['--ro', '--rw', '--intern-table']:
> > pass
> > elif key == '--output-file':
> > output_file = value
Yeah, good point, I'll send out a patch for that.
More information about the dev