Skip to content

Documentation on reducing package size and custom CFLAGS#2517

Merged
PrettyWood merged 1 commit intopydantic:masterfrom
peterroelants:allow_custom_compile_flags
Jun 4, 2021
Merged

Documentation on reducing package size and custom CFLAGS#2517
PrettyWood merged 1 commit intopydantic:masterfrom
peterroelants:allow_custom_compile_flags

Conversation

@peterroelants
Copy link
Copy Markdown
Contributor

@peterroelants peterroelants commented Mar 13, 2021 •
edited
Loading

Change Summary

  • Documentation update on how to custom compile pydantic when using pip install.
  • Small change in setup.py to allow for custom CFLAGS when compiling.

Setup.py CFLAGS

pydantic's setup.py overwrites all custom CFLAGS that a user might pass in when rebuilding pydantic.
This change appends custom CFLAGS to the current CFLAGS used when building. There should not be any change in the current default behaviour.

By appending custom CFLAGS to the end it is possible to overwrite the default optimization flags currently used in pydantic. From the gcc docs:

If you use multiple -O options, with or without level numbers, the last such option is the one that is effective.

Performance impact of setting different CFLAGS

size benchmark avg time
default pip install (from PyPI) 45M 70.7μs
pip install --no-binary (from PyPI) 796K 101.4μs
pip install --no-binary (from PyPI) with cython 6.4M 67.8μs
pip install --no-binary with cython CFLAG "-O3" (This branch) 6.4M 68.1μs
pip install --no-binary with cython CFLAG "-Os" (This branch) 5.2M 74.6μs

Related issue number

Issue #2276 will be resolved by documenting how to reduce pydantic installation size. Additionally, users can pass in custom CFLAGs to trade-off speed vs size.

Checklist

  • Unit tests for the changes exist (Already existent)
  • Tests pass on CI and coverage remains at 100%
  • Documentation reflects the changes where applicable
  • changes/<pull request or issue id>-<github username>.md file added describing change
    (see changes/README.md for details)

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 13, 2021 •
edited
Loading

Codecov Report

Merging #2517 (a2efe5b) into master (7b7e705) will not change coverage.
The diff coverage is n/a.

❗ Current head a2efe5b differs from pull request most recent head d755928. Consider uploading reports for the commit d755928 to get more accurate results

@@            Coverage Diff            @@
##            master     #2517   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           25        25           
  Lines         5109      5109           
  Branches      1050      1050           
=========================================
  Hits          5109      5109

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aca18a9...d755928. Read the comment docs.

@peterroelants peterroelants changed the title Allow custom CFLAGS and update documentation on reducing size. Documentation on reducing package size and custom CFLAGS Mar 13, 2021
@peterroelants peterroelants mentioned this pull request Mar 13, 2021
Closed
@peterroelants
Copy link
Copy Markdown
Contributor Author

peterroelants commented Apr 30, 2021

@PrettyWood I was wondering, should I request a review for this somewhere? And if so, where can I do this?
Or should I just leave it to be reviewed eventually?

@PrettyWood
Copy link
Copy Markdown
Contributor

PrettyWood commented Apr 30, 2021 •
edited
Loading

I let Samuel merge PRs. Nothing to do on your side ;)

@samuelcolvin
Copy link
Copy Markdown
Member

samuelcolvin commented Apr 30, 2021

Hopefully I can make time to work on pydantic next week.

@peterroelants
Copy link
Copy Markdown
Contributor Author

peterroelants commented Apr 30, 2021

Great, I'll leave it here then. No rush from my side.

samuelcolvin
samuelcolvin reviewed May 9, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@samuelcolvin samuelcolvin left a comment

Choose a reason for hiding this comment

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

otherwise LGTM.

Comment thread changes/2517-peterroelants.md Outdated
Comment thread docs/install.md Outdated
Comment thread docs/install.md
Comment thread setup.py
os.environ['CFLAGS'] = '-O3'
# Set CFLAG to all optimizations (-O3)
# Any additional CFLAGS will be appended. Only the last optimization flag will have effect
os.environ['CFLAGS'] = '-O3 ' + os.environ.get('CFLAGS', '')
Copy link
Copy Markdown
Member

@samuelcolvin samuelcolvin May 9, 2021

Choose a reason for hiding this comment

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

👍

@samuelcolvin
Copy link
Copy Markdown
Member

samuelcolvin commented May 9, 2021

please update.

@github-actions github-actions Bot added awaiting author revision awaiting changes from the PR author and removed ready for review labels May 9, 2021
@peterroelants
Copy link
Copy Markdown
Contributor Author

peterroelants commented May 9, 2021 •
edited
Loading

please update.

@samuelcolvin Thank you for the review. I updated this PR according to the suggestions, rebased the changes on the latest master, and pushed the changes here. This should be ready to merge. please review.

@github-actions github-actions Bot added ready for review and removed awaiting author revision awaiting changes from the PR author labels May 14, 2021
@samuelcolvin samuelcolvin mentioned this pull request May 20, 2021
Closed
PrettyWood
PrettyWood suggested changes Jun 4, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@PrettyWood PrettyWood left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Please update and rebase/merge to have new deps to fix CI.
Otherwise looks good thanks!

Comment thread docs/install.md Outdated
@PrettyWood PrettyWood added awaiting author revision awaiting changes from the PR author and removed ready for review labels Jun 4, 2021
@peterroelants
Copy link
Copy Markdown
Contributor Author

peterroelants commented Jun 4, 2021

@PrettyWood I:

  • Merged your suggestion,
  • Squashed the commits into a single commit while rebasing on the latest master,
  • Pushed changes to this branch.

Please review

@github-actions github-actions Bot added ready for review and removed awaiting author revision awaiting changes from the PR author labels Jun 4, 2021
@peterroelants peterroelants requested a review from PrettyWood June 4, 2021 18:24
@PrettyWood
Copy link
Copy Markdown
Contributor

PrettyWood commented Jun 4, 2021

https://smokeshow.helpmanual.io/4944455v3p5m500e732e/install/

Screen Shot 2021-06-04 at 8 37 38 PM

I think you need to remove the colon at the end of !!! note:
Please update

@github-actions github-actions Bot added awaiting author revision awaiting changes from the PR author and removed ready for review labels Jun 4, 2021
@peterroelants
Allow custom CFLAGS and update documentation on reducing size.
d755928 
Squashed commit Co-authored-by: Eric Jolibois <em.jolibois@gmail.com>
@peterroelants
Copy link
Copy Markdown
Contributor Author

peterroelants commented Jun 4, 2021

https://smokeshow.helpmanual.io/4944455v3p5m500e732e/install/

Screen Shot 2021-06-04 at 8 37 38 PM

I think you need to remove the colon at the end of !!! note:
Please update

resolved

please review

@github-actions github-actions Bot added ready for review and removed awaiting author revision awaiting changes from the PR author labels Jun 4, 2021
@PrettyWood PrettyWood merged commit 2e2edf4 into pydantic:master Jun 4, 2021
@zrothberg zrothberg mentioned this pull request Mar 17, 2022
Closed
@fenrir-z fenrir-z mentioned this pull request Sep 30, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@samuelcolvin samuelcolvin samuelcolvin left review comments

+1 more reviewer

@PrettyWood PrettyWood PrettyWood approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@samuelcolvin samuelcolvin

@PrettyWood PrettyWood

Labels

documentation ready for review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@peterroelants @PrettyWood @samuelcolvin