Yelp / bravado-core

Other
109 stars 98 forks source link

Tentative to remove overhead of models #375

Closed macisamuele closed 2 years ago

macisamuele commented 4 years ago

The goal of this PR is to reduce the performance gap present while unmarshaling schemas if use_models is enabled or disabled.

In order to achieve the performance parity between models disabled (so raw dict) and models enabled (so bravado_core.model.Model) I've ensured that Model class extends dict.

In order to prevent unexpected regressions I've made sure that no tests would have been modified after the changing Model base class. This is true except for the last commit (6374839) where there is a backward incompatible change (from commit message)

Remove check for additional properties while creating model This is needed to reduce even more the difference in Model vs dict creation. NOTE: Once a model is created there are no checks in case additional properties would have been added.

Some hightligths:

Raw data are available on https://gist.github.com/macisamuele/dcd58fafd470c80902643760e4eb87ea

coveralls commented 4 years ago

Coverage Status

Coverage decreased (-0.1%) to 99.054% when pulling 6374839cd07662c6db6c999bf293b0cd626ab283 on macisamuele:maci-tentative-to-remove-overhead-of-models into 2079240e9eae5dce78e2f07efab742265a619b52 on Yelp:master.

macisamuele commented 4 years ago

@sjaensch hope that you're doing well. Sorry for the long turnaround for this PR.

I've tried to resume the work on this PR, I've addressed few comments. In the next days I'll try to get back to speed with this as by running few quick benchmarks (just to revalidate the status-quo) I've noticed some small inconsistencies with the results (but I'd like to investigate a bit more before raising additional concerns)