Why do we still need tosort_dict_recursive here if we're already doing that in the dict serialization itself? (Same question for the 2 calls below)

I tried ur recommendation and it doesn't work.

It will only work when converting to Python type (to_python). Since we wrote the serializing logic.
As forto_json we use the serde package if am not wrong.
They are separate into defined field processing and non defined fields processing. Cause we need to sort before using the serde package.

This is based on my high level understanding.

I am unconvinced we need the recursive sort here either.serde will serialize the content in the order we call.serialize_entry, I think we just need to make sure that everywhere we do that we're sorting first.

Could we not do a recursive sort here, but sortnew_dict specifically after we've built it here?

I think it might be less efficient for sorting new_dict at the end? It would require an extra iteration over all items. The current approach sorts values recursively as they're being processed, which should be more optimal since we are already iterating and setting key values.

From a quick scan, I think the implementation here would have exponential blowup, because for a dict-of-dicts, when serializing the top-level dict we'll callsort_dict_recursive, and then again we'll call it for each dict when we start serializing them below.

Same as above -- I'd like to get rid of recursive sorting and do this after we build themap here. But I'm not 100% sure the data types here allow that...

We haveserialize_pairs_python andserialize_pairs_json functions ininfer.rs which are probably good starting points for code re-use which might lead to a solution.

Fully agree the recursive sort is questionable.

This check for include and exclude defined in the parameters. Those fields are definied in pydantic schema.

So this function handles the serialization or extra handling of those fields.

While this block is cater for extra fields (not defined in pydantic schema)
So we need to sort recursively for that as well.
Both of them doesn't seem to use dict.rs serializer . As the objective isto_json and using serde package directly...

https://github.com/pydantic/pydantic-core/pull/1637/files#diff-d32436e9ac9b3e5dfaf920749269f6cff3dae3b3e030561f9b9bf50447067450R540

Why a branch for lists here?

From a quick scan, I think the implementation here would have exponential blowup, because for a dict-of-dicts, when serializing the top-level dict we'll callsort_dict_recursive, and then again we'll call it for each dict when we start serializing them below.

There's now a lot of code duplication in this file. It feels like what we really need is afor_all_fields function which takes a closure and calls it for each field to serialize in turn.

That function can take responsibility for, if sorting requested, collecting all fields into a vec and sorting them first. Otherwise can just do the serializing in a streaming fashion.

I am unconvinced we need the recursive sort here either.serde will serialize the content in the order we call.serialize_entry, I think we just need to make sure that everywhere we do that we're sorting first.

There are places in this file where we serialize dicts, those presumably need sorting added (and tests). Best way to test those code paths is by using theserialize_as_any=True runtime flag.

Oic, i just realized that the fields.rs json serialization uses infer.rs ... thats a great hint. Managed to implement it. Will look at adding the tests... Thanks David.

We haveserialize_pairs_python andserialize_pairs_json functions ininfer.rs which are probably good starting points for code re-use which might lead to a solution.

Fully agree the recursive sort is questionable.

We need to add tests for computed fields; at the moment the implementation neither supports them nor tests them.