[issue28398] Return singleton empty string in _PyUnicode_FromASCII

[Xiang Zhang]

Xiang Zhang added the comment:

> My question is simple: in what circumstances the patch has an effect?

My original intention is that there is no need for the caller to check for the 
empty string. Even there is no use case now, it could be in future.

But Serhiy, I am actually very glad to get a determined result, even it's 
rejected. If the issue is still open, I would think you are still open to 
discussion. Such a trivial patch isn't worth it.

--
assignee:  -> serhiy.storchaka

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28398] Return singleton empty string in _PyUnicode_FromASCII

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

My question is simple: in what circumstances the patch has an effect?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28398] Return singleton empty string in _PyUnicode_FromASCII

[STINNER Victor]

STINNER Victor added the comment:

> We should know what is the benefit of the patch before committing it.

The purpose of the patch is to use the empty string singleton to reduce the 
memory footprint, instead of creating multiple empty (Unicode) string objects.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28398] Return singleton empty string in _PyUnicode_FromASCII

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

I know that _PyUnicode_FromASCII() is called with size=0. But most callers call 
it only with size!=0. I didn't investigate further. We should know what is the 
benefit of the patch before committing it. Just "for consistency" is a weak 
argument itself.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28398] Return singleton empty string in _PyUnicode_FromASCII

[STINNER Victor]

STINNER Victor added the comment:

To reduce the memory footprint, it's important that we reuse internal Unicode 
singletons. For the empty string singleton, you have two choices:

* if length is equal to zero, return the singleton
* create a string and then call unicode_result()

unicode_result() should be used when the output length is not easy to compute 
or when the code is complex.

Here is the code is very simple and the output length is obvious.

Serhiy: "In what circumstances _PyUnicode_FromASCII is called with size=0?"

I checked the current code: I'm unable to see any function which can call 
_PyUnicode_FromASCII() with size=0.


Xiang: "IMHO, _PyUnicode_FromASCII is a private API and could be used in other 
places in future. We should not rely on the caller to check and return the 
singleton empty string."

I agree. Xiang's patch is short and makes sense, so I vote +1 on it.

I let Serhiy decides if the patch should be merged or not.

If we decide to reject the patch, I suggest to add the following code at the 
beginning of _PyUnicode_FromASCII:

/* Detect missed optimization */
assert(size != 0);

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28398] Return singleton empty string in _PyUnicode_FromASCII

[Xiang Zhang]

Xiang Zhang added the comment:

IMHO, _PyUnicode_FromASCII is a private API and could be used in other places 
in future. We should not rely on the caller to check and return the singleton 
empty string.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28398] Return singleton empty string in _PyUnicode_FromASCII

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

The case for size=0 often is handled before calling _PyUnicode_FromASCII. In 
what circumstances _PyUnicode_FromASCII is called with size=0? Is this case 
covered by tests?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28398] Return singleton empty string in _PyUnicode_FromASCII

[Xiang Zhang]

Xiang Zhang added the comment:

The cost is really small (an integer compare vs memory malloc and copy). The 
advantages are fast path for empty strings and retaining consistency with other 
codes in unicodeobject.c. You can see other places use the same optimization, 
e.g. PyUnicode_FromUnicode, PyUnicode_FromUCS1, and some functions using 
unicode_result.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28398] Return singleton empty string in _PyUnicode_FromASCII

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Additional check has a non-zero cost. What is the benefit of this change?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28398] Return singleton empty string in _PyUnicode_FromASCII

[Xiang Zhang]

New submission from Xiang Zhang:

Right now in _PyUnicode_FromASCII, it only optimises for size 1 not size 0. All 
other places getting the same situation in unicodeobject.c do both.

--
files: _PyUnicode_FromASCII.patch
keywords: patch
messages: 278364
nosy: haypo, serhiy.storchaka, xiang.zhang
priority: normal
severity: normal
stage: patch review
status: open
title: Return singleton empty string in _PyUnicode_FromASCII
type: enhancement
versions: Python 3.7
Added file: http://bugs.python.org/file45038/_PyUnicode_FromASCII.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com