This has been brought up in NumPy, NumPy subclasses bytes, and I am not immediately sure how to best initialize the hash value. We can't really call __new__ from C, but FromString would give the wrong type.

Would it make sense to move the -1 initialization to a custom tp_alloc? Or am I missing an obvious solution? If this the slow is removed entirely, then it does not matter of course.

This has been brought up in NumPy, NumPy subclasses bytes, and I am not immediately sure how to best initialize the hash value.

Would you give me a link to the code?

Would it make sense to move the -1 initialization to a custom tp_alloc? Or am I missing an obvious solution? If this the slow is removed entirely, then it does not matter of course.

How about tp_init?

@methane the code is here:
https://github.com/numpy/numpy/blob/46a61b3aed3cf674586b456b05c6c9b7b9c5020d/numpy/core/src/multiarray/scalarapi.c#L687

With the initialization a bit later.

The point being, this creates a bytes subclass the manual way currently. I am not condoning that, but I am also not sure there is an obvious replacement for creating a byte subclass instances. FromStringAndSize would be the right thing, but that won't give the subclass.

EDIT: The point about init/new is that they are Python level API, and as such expect args/kwargs. And I don't really think that would be ideal here either. I don't mind this requiring a bit of a hackish solution, but right now I am not sure what the solution is at all.

Thank you. tp_alloc seems good.

What are problems with calling __new__ (tp_new)?

@serhiy-storchaka it requires passing a Python object to create the new bytes (subclass) instance. NumPy does not have such an object available. We would have to create it just to call bytes.__new__()... There is a reason the C-API provides PyBytes_FromStringAndSize, but we can't use it for a subclass.

Now, I don't care about using "unstable" API. So if the category of "unstable" is created (and the discussion and Petr's last mail sound a bit like it), then it would make sense to me if this ABI detail was available only through that? Otherwise a custom tp_alloc initializing this to -1 makes perfect sense to me, but maybe there are e.g. micro-optimization reasons against it.

Current ideas:

• Create custom .tp_alloc()
• Add new PyTypeObject *type parameter to _PyBytes_FromSize() and expose it as a cpython API.

BTW, Since bytes object is PyVarObject, every subclasses won't extend the PyBytesObject, am I right?

BTW, Since bytes object is PyVarObject, every subclasses won't extend the PyBytesObject, am I right?

NumPy certainly does not, I suppose you could dynamically add more "items" at the end in a custom tp_alloc, though? Again, I am happy with changes or removal of ob_shash (so long it is not in a bug-fix release).
It just would be nice to avoid the DeprecationWarning in a clean way

The happy path of the code in question is:

    PyTypeObject *type = /* get the type object */;
    int itemsize = /* get the string 'length' */;
    PyObject *obj;
    void *destptr;
    obj = type->tp_alloc(type, itemsize);
    destptr = PyBytes_AS_STRING(obj);
    ((PyBytesObject *)obj)->ob_shash = -1;
    memcpy(destptr, data, itemsize);
    return obj;

How should NumPy refactor that?

I recommend keeping the ob_shash field. The memory use is negligible — 25% on tiny bytes instances and diminishing further in significance as the sizes get larger.

The caching benefit doesn't happen often but when it does the benefits are significant. Crypto hashes are always stored as bytes and we use them to check for duplicates (i.e. the way git works). The output of str.encode() is bytes and it is legitimate to use this as a cache key (with the lru_cache for example). If I recall correctly, DNA sequence fragments get stored as bytes and are used for repeated dict/set lookups.

This code has been stable for a very long time and it is almost certain that some code in the wild depends on cached hash values for efficient computation. ISTM the deprecation damages that code for no good reason.

Looking back over the thread, it seems that the original motivations are mostly gone or are insignificant. Unless someone really wants this, I request that the field be undeprecated.

Memory saving is not the only reason for removing the hash cache.
Please read my previous comment.

Additionally, this issue doesn't remove the hash cache.
This issue only deprecate direct access to PyBytesObjects.ob_shash.
Even if we won't remove the hash cache, direct access to struct member is not recommended.

The hash cache should remain. If you want to block direct access to the structure member that is reasonable.

For code that does use the hash cache, it is performance critical. The feature has been in a place a long time. Code in the wild almost certainly relies on it (and reasonably so) for good performance. Saying "that CPUs are fast" is dismissive and ignore the essential fact that bytes can be of any size. At some size (likely somewhat small), a fast CPU computation loses to a field lookup, especially for a field that is likely already loaded in the cache line. In contrast, the data array is much less likely to be in the current cache line, so its memory accesses will be expensive.

I've ready you "reasons" and still object to removing the hash cache. Please undo the deprecation.