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

refactor: squash modules into one to enable optimizations #16

Merged
merged 1 commit into from
Aug 22, 2024
Merged

Conversation

kszucs
Copy link
Owner

@kszucs kszucs commented Aug 21, 2024

At this point the benchmarks show impressive results:

koerce/tests/test_y.py::test_pydantic PASSED
koerce/tests/test_y.py::test_annotated PASSED


-------------------------------------------------------------------------------------------- benchmark: 2 tests -------------------------------------------------------------------------------------------
Name (time in ns)            Min                     Max                  Mean              StdDev                Median                IQR            Outliers  OPS (Kops/s)            Rounds  Iterations
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
test_annotated          512.7001 (1.0)        3,164.3000 (1.0)        596.8280 (1.0)      148.8698 (1.0)        561.4397 (1.0)      38.2401 (1.0)      802;2463    1,675.5245 (1.0)       20000          50
test_pydantic         1,223.6397 (2.39)     105,432.3203 (33.32)    1,447.6836 (2.43)     978.5593 (6.57)     1,318.7603 (2.35)     95.6000 (2.50)     806;2200      690.7587 (0.41)      20000          50
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------



concatenate_files(["builders.py", "patterns.py", "annots.py"], "_internal.pyx")
extension = Extension("koerce._internal", ["koerce/_internal.pyx"])
Copy link
Owner Author

Choose a reason for hiding this comment

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

I wasn't able to cimport from pure python cythonized modules even with agumenting pxd files, so I started to move over to pure cython syntax, but the development time has increased which made me value fast iterations during development even more.
So I had to come up with a solution so that the implementation is being split into modules (easier maintanence) but still profit from the timing. The solution was to squash all the python files into a single one before cythonizing, in order to make it work the internal imports must be removed.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess we'll see whether this becomes a burden long term. For now, the solution seems good!

Copy link
Owner Author

Choose a reason for hiding this comment

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

There is already a feature request on cython's side, hopefully this will be fixed in the meantime cython/cython#4892

Copy link
Collaborator

@cpcloud cpcloud left a comment

Choose a reason for hiding this comment

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

Seems legit, if slightly annoying that this is required.

@kszucs kszucs merged commit 1788181 into main Aug 22, 2024
24 checks passed
@cpcloud cpcloud deleted the fastinit branch August 22, 2024 11:26
@kszucs kszucs restored the fastinit branch August 24, 2024 17:58
@kszucs kszucs deleted the fastinit branch August 24, 2024 17:59
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.

2 participants