CodSpeed Performance Report

Merging#10868 willnot alter performance

_{ComparingMarkusSintonen:fast-setattr (404b8b7) withmain (30ee4f4)}

Summary

✅ 44 untouched benchmarks

🆕 2 new benchmarks

Benchmarks breakdown

Coverage report

Click to see where and how coverage changed

File	Statements	Missing	Coverage	Coverage (new stmts)	Lines missing
pydantic
main.py
pydantic/_internal
_model_construction.py
Project Total

This report was generated bypython-coverage-comment-action

please review

Consider the PR approved by me, at least conceptually

Fantastic, thanks for the prompt review.

I've give this a nit-picky review this evening, then we can move forward!

This makes me think more about what else we could potentially memoize in the schema gen department...

One other thing I want to make sure of - this doesn't leave us with any pickling issues? I don't think so, given passing tests, but we should check.

Thanks, I think this is a smart idea. We might have to worry about the size of the cache for large models (with a lot of fields). If we encounter such issues, we could use proper functions defined once instead of creating lambdas everytime

I wouldnt worry about size of it as anyways all the fields are listed in various ClassVars. However if we want to remove the tiny overhead of field name strs we could push the handler fn into eg FieldInfo/ModelPrivateAttr.

functions defined once

I purposely didnt want to touch the model generation side to not make it anymore heavier than it already is. Because of the mentioned large models it could just do work for no good reason in case fields are not even used like this.

Not sure exactly what you mean byfield name strs /didnt want to touch the model generation side, but what I wanted to say is we could do something like this:

HANDLERS= {'descriptor':lambdam,val:attribute.__set__(m,val),'cached_property':lambdam,val:m.__dict__.__setitem__(name,val),    ...}def_setattr_handler(name:str,value:Any):    ...ifhasattr(attribute,'__set__'):returnHANDLERS['descriptor']    ...elifisinstance(attr,cached_property):returnHANDLERS['cached_property']

So that we don't create a new function every time.

but what I wanted to say is we could do something like this

Ah yes I see! That would make sense yes

@Viicos the suggestion now done here7dfbe50