Is this necessary?
Doesn't the profiler extract the same data from the builtin function that it could from the method descriptor?

This piece is copied from the newsetprofile I believe. The idea behind it is to make sureget_cfunc_from_callable only returns aPyCFunctionObject. If it's not, then((PyCFunctionObject *)cfunc)->m_ml won't work.

What do you propose here? Simply returncallable? We need to check that anyway becauseCALL event can be triggered before calling a Python function and we don't want to add profiler entry on that(it should be dealt with later). I did realize thatPy_RETURN_NONE was incorrect -NULL should be returned.

All that happens to the builtin function objects is it gets passed down tonormalizeUserObj() which then does some elaborate lookup to get the method descriptor back again. Both the method descriptor and builtin function contain a pointer to the samePyMethodDef struct.

So, leave the method descriptor alone here, and innormalizeUserObj() create the same string that as would be created for the builtin function.

Themo objecton line 175 is the method descriptor.

There are three purposesget_cfunc_from_callable needs the serve:

1. To get the object(callable) to print later, which could be resolved innormalizeUserObj() like you said.
2. To get a unique key for the hash table(actually a binary tree). In this case,((PyCFunctionObject *)cfunc)->m_ml was used, maybe the callable itself could be used as well? Not sure whym_ml was chosen at the first place, the builtin functions probably have dinstinct addresses anyway? Even for the builtin methods(likelist.append), using(void*)callable directly might work?
3. To filter out unwanted data. This is why this part is necessary. Without trying to get the C function from the method, how could we know if this is a call to a builtin function that we would like to log? It could be a simple Python call(f()), which would triggerCALL event. It could be a method call, but not to a C function(self.some_method()). We need to filter these entry out before we even reachnormalizeUserObj()(that's where we are about to log the function call).

The current implementation is trying to swap out thesetprofile layer without touching the internal profiling system. It's true that the profiling system could be optimized, but it's also risky and probably need extra care. If we want to land this in 3.12, maybe we should avoid changing the profiling logic for now.

I agree with your point about risk.

I think it is worth cleaning up the internals. We couldn't do it before, as the conversion from method descriptor to builtin function occurred before cprofile got to see it. Maybe we can get it done for 3.12, maybe not.

So let's get this change in for 3.12, and we can streamline things later.

We can clean up the internals, but we still need to address point 3. We need to filter out entries that we don't want, and that probably requires resolving the descriptors.

All the information in the fake builtin function is also in the original method descriptor, so whatever the filter was doing should still work.

Do you suggest that we can get rid ofPy_TYPE(callable)->tp_descr_get(callable, self_arg, (PyObject*)Py_TYPE(self_arg)) because we don't need the actualPyCFunctionObject in it? We still need to check againstPy_TYPE(callable) == &PyMethodDescr_Type for the actual "builtin methods" right? To filter out Python defined methods?

Why has this line been removed?

Because what the test was partially testing won't work anymore. The line relies on the fact that the original obj was deallocated when replace - that won't happen because it's still being referenced by the monitors.

Having that line would also cause an infinite exception loop, probably due to the new mechanism.

Sorry not infinite, but very frequent warning. The issue is we did not disable the profiling - that's what I mentioned in the description - enabling the profiling with one object and disable it with another won't work.