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

Add support for generating protobuf3 definition #166 #1085

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

kapilreddy
Copy link

Closes #166

Contribution checklist

  • Tests verified by ./bin/kaocha and ./bin/node
  • malli.perf.core/quick-bench on malli.protobuf3-schema/transform with a fairly complex schema takes 28.656211 µs

Overview of change

  1. Add malli.protobuf3-schema which converts malli schema to protobuf3 defintions
  2. Add tests covering nested data structures
  3. Add perf test for protobuf3-schema
  4. Add fixes for bin/node and malli.perf.core

The commit introduces Protocol Buffer 3 (protobuf3) support for Malli schemas:

- Implement transform function to convert Malli schemas to protobuf3
- Add transform-schema function for handling nested structures
- Add support for enum types in Malli schemas
- Add comprehensive test suite for transform and transform-schema functions
- Include tests for deeply nested structures and complex schemas
- Ensure generated protobuf3 definitions are syntactically correct
- Maintain same input/output interface for compatibility
- Eliminate mutable state from the collect-definitions
function, making it a pure function
malli.perf.core/serve! uses clj-async-profiler/serve-files which is
a depcrecated function

Updated the code to use clj-async-profiler/serve-ui

clojure-goes-fast/clj-async-profiler@733ae1e

clj-async-profiler version was updated here,
metosin@9d705eb
User cherry-none fn instead of cherry in * case
@ikitommi
Copy link
Member

ikitommi commented Aug 7, 2024

Hi, and thanks for the PR! Quick comments:

  1. the solution should be extendable, multimethods are a great way for doing this
  2. There could be a property-based way to define the mappings, e.g. [:fn {:protobuf/type "double"} #(double? %)]
  3. all default schemas should be mapped if they have protobuf3 counterpart, e.g. (protobuf/transform :keyword) fails currently

I think the solution could look much like the JSON Schema transformation:

what do you think?

@kapilreddy
Copy link
Author

Makes sense. I'll address these comments.

@ikitommi
Copy link
Member

ikitommi commented Aug 8, 2024

for perf testing, most of the time goes in constructing the schema. It can be cached to check the actual transformation code perf. So:

(def schema
  (m/schema
   [:map
    [:id string?]
    [:metadata [:map
                [:created_at inst?]
                [:tags [:vector string?]]]]
    [:data [:vector [:map
                     [:name string?]
                     [:details [:map
                                [:type [:enum :type-a :type-b :type-c]]
                                [:properties [:vector [:map
                                                       [:key string?]
                                                       [:value [:or string? int? boolean?]]
                                                       [:nested [:vector [:map
                                                                          [:sub_key string?]
                                                                          [:sub_value any?]]]]]]]]]]]]]))

(defn transform-perf []
  ;; 28.656211 µs -> 640ns
  (p/bench (protobuf/transform schema)))

@kapilreddy
Copy link
Author

That's a good catch! I'll fix the perf namespace as well.

@ikitommi ikitommi marked this pull request as draft August 30, 2024 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

Support for Protobuf3 definition/schema
3 participants