-
Notifications
You must be signed in to change notification settings - Fork 262
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
Allocations overhead ? #843
Comments
Thanks so much for creating the issue, really interesting. We were discussing @davidhewitt what do you think? |
What was your benchmark of? Validation or validator construction? |
It is something similar to this class ComplexPydanticModel(BaseModel):
# I also tried `defer_build=True` or removing some of `validate_xxx` but didn't make a noticeable difference
model_config = ConfigDict(from_attributes=True, validate_default=True, validate_return=True)
# fields/validators are omitted
class Response(BaseModel, Generic[T]):
data: T
BenchmarkModel = Response[List[ComplexPydanticModel]]
data = {"data": data}
for _ in range(1000):
BenchmarkModel.model_validate(data)
|
Thanks for the report and the proposed patch. Inspired by it I'm going a bit further and reducing away many cases where we clone validators (using probably just I've got a patch which is almost there but I'm aiming to merge #867 first to make the patch simpler. |
First of all, thanks for the fantastic library.
While benchmarking pydantic v2 on some complex models with a lot of fields/nesting (unfortunately I can't share the models) I noticed abnormal performance due to allocations/deallocations:
@model_validator(mode='wrap')
is used in these models but is dwarfed by theClone
andDrop
calls. The (de-)allocations represent ~75% of the total run time.I tried replacing the
Box<CombinedValidator>
with anArc<RwLock<CombinedValidator>>
for:FunctionWrapValidator.validator
FunctionBeforeValidator.validator
FunctionAfterValidator.validator
After rebuilding with the patch, the (de-)allocations calls dropped:
Benchmark (1000 iterations)
Box<CombinedValidator>
Arc<RwLock<CombinedValidator>>
But I'm not sure if this is actually sound (ref):
There's no
set_ref
in the code base anymore, but I guess the same concern is still valid forvalidator.complete()
?Selected Assignee: @samuelcolvin
The text was updated successfully, but these errors were encountered: