On 20Oct2018 0901, Stefan Behnel wrote:

I'd be happy to get rid of it. But regarding the use under Windows, I wonder if there's interest in keeping it as a special Windows-only feature, e.g. to speed up the data exchange with the Win32 APIs. I guess it would have to provide a visible (performance?) advantage to justify such special casing over the code removal.

I think these cases would be just as well served by maintaining the original UCS-2 representation even if the maximum character fits into UCS-1, and only do the conversion when Python copies the string into a new location. I don't have numbers, but my instinct says the most impacted operations would be retrieving collections of strings from the OS (avoiding a scan/conversion for each one), comparisons against these collections (in-memory handling for hash/comparison of mismatched KIND), and passing some of these strings back to the OS (conversion back into UCS-2). This is basically a glob/fnmatch/stat sequence, and is the main real scenario I can think of where Python's overhead might become noticeable. Another option that might be useful is some way to allow the UCS-1/4 <-> UCS-2 conversion to occur outside the GIL. Most of the time when we need to convert we're about to release the GIL (or have just recovered it), so even without the cache we could probably recover some of the performance impact in parallelism. (That said, these are often tied up in conditions and generated code, so it may not be as easy to do this as retaining the original format.) Some sort of tracing to see how often the cache is reused after being generated would be interesting, as well as how often the cache is being generated for a string that was originally in UCS-2 but we changed it to UCS-1. Cheers, Steve

On 22Oct2018 0413, Victor Stinner wrote:

For code like "for name in os.listdir(): open(name): ...." (replace listdir with scandir if you want to get file metadata), the cache is useless, since the fresh string has to be converted to wchar_t* anyway, and the cache is destroyed at the end of the loop iteration, whereas the cache has never been used...

Agreed the cache is useless here, but since the listdir() result came in as wchar_t we could keep it that way (assuming we'd only be changing it to char), and then there wouldn't have to be a conversion when we immediately pass it back to open(). That said, I spent some time yesterday converting the importlib cache to use scandir and separate caches for dir/file (to avoid the stat calls) and it made very little overall difference. I have to assume the string manipulation dominates. (Making DirEntry lazily calculate its .path had a bigger impact. Also, I didn't try to make Windows flush its own stat cache, and accessing warm files is much faster than cold ones.)

I'm not saying that the cache is useless. I just doubt that it's so common that it really provide any performance benefit.

I think that it is mostly useless, but if we can transparently keep many strings "native" size, that will handle many of the useful cases such as the single-use pass-through scenario like above. Cheers, Steve

On 22Oct2018 0913, Victor Stinner wrote:

Le lun. 22 oct. 2018 à 15:08, Steve Dower a écrit :

...

Agreed the cache is useless here, but since the listdir() result came in as wchar_t we could keep it that way (assuming we'd only be changing it to char), and then there wouldn't have to be a conversion when we immediately pass it back to open().

Serhiy wants to remove the cache which should *reduce* Python memory footprint on Windows.

You are proposing to fill the cache eagierly, that would increase the Python memory footprint :-/ Your proposed change is an optimisation, a benchmark is needed to see the benefit. I expect no significant difference on benchmarks of https://pyperformance.readthedocs.io/ ...

Yes, that's true. But "should reduce ... footprint" is also an optimisation that deserves a benchmark by that standard. Also, I'm proposing keeping the 'kind' as UCS-2 when the string is created from UCS-2 data that is likely to be used as UCS-2. We would not create the UCS-1 version in this case, so it's not the same as prefilling the cache, but it would cost a bit of memory in exchange for CPU. If slicing and concatentation between matching kinds also preserved the kind, a lot of path handling code could avoid back-and-forth conversions. The import benchmarks ought to be improved on Windows by this new optimisation, as this is a prime case where we regularly convert strings from what the OS gave us into UCS-1 and back into what the OS expects. But if you don't run the benchmarks on all OS's, then sure, you won't see any difference :) Cheers, Steve

On 22Oct2018 0928, Victor Stinner wrote:

...

Also, I'm proposing keeping the 'kind' as UCS-2 when the string is created from UCS-2 data that is likely to be used as UCS-2.

Oh. That's a major change in the PEP 393 design. You would have to modify many functions in CPython. Currently, the PEP 393 requires that a string always use the most efficient storage, and many optimizations and code paths rely on that assumptions.

I don't know that it requires that many modifications - those functions already have to handle UCS-2 content anyway (e.g. if I get a path from scandir() that includes a non-ASCII character), and they're only using the assumption of most efficient storage to determine the resulting storage size of a string operation (which I'm proposing should also be UCS-2 when the source strings are UCS-2, since that's the best indicator we have that it'll be used as UCS-2 later, as well as being the current implementation :) ).

I'm against this change.

Moreover, it's hard to guess how a string will be used later...

Agreed. There are some heuristics we can use, but it's definitely only a guess. That's the nature of this problem - guessing that it *won't* be used as UCS-2 later on is also a guess. Cheers, Steve

On 22Oct2018 1007, Serhiy Storchaka wrote:

22.10.18 16:24, Steve Dower пише:

...

Yes, that's true. But "should reduce ... footprint" is also an optimisation that deserves a benchmark by that standard. Also, I'm proposing keeping the 'kind' as UCS-2 when the string is created from UCS-2 data that is likely to be used as UCS-2. We would not create the UCS-1 version in this case, so it's not the same as prefilling the cache, but it would cost a bit of memory in exchange for CPU. If slicing and concatentation between matching kinds also preserved the kind, a lot of path handling code could avoid back-and-forth conversions.

Oh, I afraid this will complicate the whole code of unicodeobject.c (and several other files) a much and can introduce a lot of subtle bugs.

For example, when you search a UCS2 string in a UCS1 string, the current code returns the result fast, because a UCS1 string can't contain codes

...

0xff, and a UCS2 string should contain codes > 0xff. And there are many such assumptions.

That doesn't change though, as we're only ever expanding the range. So searching a UCS2 string in a UCS2 string that doesn't contain any actual UCS2 characters is the only case that would be affected, and whether that case occurs more than the UCS2->UCS1->UCS2 conversion case is something we can measure (but I'd be surprised if substring searches occur more frequently than OS conversions). Currently, unicode_compare_eq exits early when the kinds do not match, and that would be a problem (but is also easily fixable). But other string operations already handle mismatched kinds. Cheers, Steve

22.10.18 23:41, Steve Dower пише:

That said, I didn't remove the wchar_t cache (though I tried some tricks to avoid it), so it's possible that once that's gone we'll see an avoidable regression here, but on its own this doesn't contribute much.

Could you please test PR 2599 on Windows? It makes PyUnicode_AsWideChar() and PyUnicode_AsWideCharString() not filling the wchar_t cache. If there is a significant regression in any benchmark or small regressions in several benchmarks we can consider to continue to fill the cache in these function and to preserve the cache in future. Otherwise I'll merge the PR as is. https://github.com/python/cpython/pull/2599

Hi Serhiy, +1 to remove wchar_t cache. IMHO it wastes memory for no real performance gain. By the way, can we start to schedule the *removal* of the Py_UNICODE API? For example, decide when Py_DEPRECATED is used in the C API? Should we start to deprecate when Python 2 reachs its end of life? Or can we *remove* this API as soon as Python 2 is dead? (Please, don't use "Python 4" as a milestone to introduce such backward incompatible change.) Le sam. 20 oct. 2018 à 15:05, Stefan Behnel a écrit :

I'd be happy to get rid of it. But regarding the use under Windows, I wonder if there's interest in keeping it as a special Windows-only feature, e.g. to speed up the data exchange with the Win32 APIs. I guess it would have to provide a visible (performance?) advantage to justify such special casing over the code removal.

If someone wants to *keep* the cache, I would like to see: * statistics on the cache usage: how many strings are converted to wchar_t*? What is the percentage on all strings? * what is the cache hit? * what is the overhead of removing the cache? (on length 10, 100, 1000, 1 MB, 10 MB?) When you look at the performance of the str type, it's common that timings are smaller than 1 us. Each str function has been highly optimized. IMHO the conversion of a string to wchar_t* is cheap, especially because I expect that all strings used with the Windows API are shorter than 1,000 characters. Victor

20.10.18 16:01, Stefan Behnel пише:

But regarding the use under Windows, I wonder if there's interest in keeping it as a special Windows-only feature, e.g. to speed up the data exchange with the Win32 APIs. I guess it would have to provide a visible (performance?) advantage to justify such special casing over the code removal.

This is an interesting question, and we should found the answer right now. Should PyUnicode_AsWideChar() and PyUnicode_AsWideCharString() continue to attach the wchar_t representation to the unicode object on Windows? The cost is higher memory consumption and slower first time call. The benefit is faster following calls for the same object. Although they still need to copy the content, so the time is still O(N), just with smaller constant multiplier.