Skip to content

use default allocator #662

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Jun 8, 2023
Merged

use default allocator #662

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
dbdf001
Use default allocator in benchmarks
adriangb Jun 8, 2023
f347793
change feature name
adriangb Jun 8, 2023
ab2e503
cleanup
adriangb Jun 8, 2023
51bab87
cleanup
adriangb Jun 8, 2023
40adca5
fix feature name
adriangb Jun 8, 2023
ac54212
Update .github/workflows/codspeed.yml
adriangb Jun 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion .github/workflows/codspeed.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,12 @@ jobs:
- name: cache rust
uses: Swatinem/rust-cache@v2

- run: pip install . -v
- name: Install pydantic-core
# build-prod does not include MiMalloc because it bypasses Maturin
run: |
pip install -e .
make build-prod
Copy link
Collaborator

@dmontagu dmontagu Jun 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I thought about it more and realized I think the make build-dev thing doesn't work for me locally if it isn't already pip installed in editable mode. (Which is often an issue for me because I switch between released version and locally installed versions sometimes.) Maybe try:

Suggested change
make build-prod
pip install -e .
make build-prod

Copy link
Member Author

Choose a reason for hiding this comment

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

How will we know which version is installed then 🥲

Copy link
Member Author

Choose a reason for hiding this comment

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

Similarly, I guess we need to confirm that the MiMalloc version is what wheels are using

Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure if it's worth it, but I would imagine you could add an attribute to the pydantic_core._pydantic_core module conditionally based on the mimalloc feature flag? (And then check for it when uploading the wheel if necessary?)

diff --git a/pydantic_core/_pydantic_core.pyi b/pydantic_core/_pydantic_core.pyi
index 6859a16..517d507 100644
--- a/pydantic_core/_pydantic_core.pyi
+++ b/pydantic_core/_pydantic_core.pyi
@@ -21,6 +21,7 @@ from _typeshed import SupportsAllComparisons
 
 __all__ = (
     '__version__',
+    'allocator',
     'build_profile',
     'SchemaValidator',
     'SchemaSerializer',
@@ -35,6 +36,7 @@ __all__ = (
     'list_all_errors',
 )
 __version__: str
+allocator: str
 build_profile: str
 
 T = TypeVar('T', default=Any, covariant=True)
@@ -167,7 +169,8 @@ class Url(SupportsAllComparisons):
     def query(self) -> 'str | None': ...
     @property
     def fragment(self) -> 'str | None': ...
-    def __init__(self, url: str) -> None: ...
+    @staticmethod
+    def __new__(cls, url: str) -> Url: ...
     def unicode_host(self) -> 'str | None': ...
     def query_params(self) -> 'list[tuple[str, str]]': ...
     def unicode_string(self) -> str: ...
diff --git a/src/lib.rs b/src/lib.rs
index 613220a..9e9229e 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -42,9 +42,20 @@ pub fn get_version() -> String {
     version.replace("-alpha", "a").replace("-beta", "b")
 }
 
+#[cfg(feature = "mimalloc")]
+fn get_allocator() -> String {
+    "mimalloc".to_owned()
+}
+
+#[cfg(not(feature = "mimalloc"))]
+fn get_allocator() -> String {
+    "default".to_owned()
+}
+
 #[pymodule]
 fn _pydantic_core(_py: Python, m: &PyModule) -> PyResult<()> {
     m.add("__version__", get_version())?;
+    m.add("allocator", get_allocator())?;
     m.add("build_profile", env!("PROFILE"))?;
     m.add_class::<PySome>()?;
     m.add_class::<SchemaValidator>()?;

(Haven't tested; also maybe it's overkill)

Copy link
Member Author

Choose a reason for hiding this comment

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

Looks like it’s using the default allocator for benchmarks now based just on the slowdowns 😂.

I was thinking something similar but it’s complex. How do you feel about moving forward without that and add ing it later if it’s really needed?

python -c 'import pydantic_core'
env:
CONST_RANDOM_SEED: 0 # Fix the compile time RNG seed

Expand Down
4 changes: 3 additions & 1 deletion Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,9 @@ crate-type = ["cdylib", "rlib"]
extension-module = ["pyo3/extension-module"]
# required for cargo bench
auto-initialize = ["pyo3/auto-initialize"]
default = ["mimalloc", "mimalloc/local_dynamic_tls", "pyo3/generate-import-lib", "pyo3/num-bigint"]
# disabled for benchmarks since it makes microbenchmark performance more flakey
mimalloc-allocator = ["mimalloc", "mimalloc/local_dynamic_tls"]
default = ["pyo3/generate-import-lib", "pyo3/num-bigint"]

[profile.release]
lto = "fat"
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ Source = 'https://github.com/pydantic/pydantic-core'
[tool.maturin]
module-name = "pydantic_core._pydantic_core"
bindings = 'pyo3'
features = ["pyo3/extension-module"]
features = ["pyo3/extension-module", "mimalloc-allocator"]

[tool.ruff]
line-length = 120
Expand Down
2 changes: 1 addition & 1 deletion src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ extern crate core;

use pyo3::prelude::*;

#[cfg(feature = "mimalloc")]
#[cfg(feature = "mimalloc-allocator")]
#[global_allocator]
static GLOBAL: mimalloc::MiMalloc = mimalloc::MiMalloc;

Expand Down