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

Investigate moving away from reflect and to a auto-generated API #198

Open
alexanderConstantinescu opened this issue Jul 15, 2021 · 1 comment

Comments

@alexanderConstantinescu
Copy link
Contributor

libovsdb uses the reflect package quite extensively to define a generic API for all models defined by the OVS DB schema.

reflect however is:

  • quite inefficient in terms of performance
  • difficult to read and develop
  • could lead to runtime errors which were undetected at compile time.

The original reasons for using reflect was to not clutter the API with a lot of duplicated code (for different models) which had to be manually maintained. However, there should be ways to be able to automatically generate the API code, similar to what is done by modelgen for the DB representation. I emphasis should as I have not delved into the details of the OVS DB model to actually study its quirks and details (which might / might not make this feasible).

Doing this would most likely also re-define the API of libovsdb, which might be something unwanted. I am however opening this issue in a way to get a discussion going on the topic.

For reference: Kubernetes uses client-gen generated code for its API, which then allows it to stay away from reflect

@dave-tucker
Copy link
Collaborator

Thanks for the suggestion @alexanderConstantinescu

The drawbacks of reflect are clear, however, we make a trade off against those drawbacks for usability.
To your points:

  1. For performance, the penalty we pay is small. You can see from Add libovsdb Benchmarks to CI #80 (or even from more recent pprof's) that the majority of CPU time is spent on JSON marshal/unmarshaling and the reflect overhead is comparatively small.
  2. It's awful to read/develop, but thankfully that stays contained in libovsdb and doesn't proliferate in to generated code that would be maintained by library users - e.g ovn-k8s
  3. I agree that runtime errors suck and I'd like as much to be done at compile time as possible. I'm hopeful that we can get there by iterating on the existing API.

However, there should be ways to be able to automatically generate the API code, similar to what is done by modelgen for the DB representation.

There is, but your generated files would essentially in-line at lot of the generic things in ovsdb.bindings and mapper. The surface of the generated code would be very large and you still won't be totally free of reflection or runtime type assertions.
So overall, I"m not sure it's worth it.

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

No branches or pull requests

2 participants