Skip to content

Commit

Permalink
locale.c: Remove no-longer needed parameter and computation
Browse files Browse the repository at this point in the history
When a locale category changes, LC_ALL necessarily also does.  When we
are looping changing a bunch of categories, LC_ALL's intermediate values
are of no importance; only the final value matters.  Prior to this
commit, this was handled by setting a flag to not do the recalculation
until the final loop iteration.

But a more elegant solution is to not recalculate it all, until the
value is actually needed.  So the code is changed to invalidate the
stored value if an individual locale category changes, and then do the
calculation only when needed and invalidated.
  • Loading branch information
khwilliamson committed May 6, 2023
1 parent 17b76da commit 25b59bf
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 69 deletions.
4 changes: 1 addition & 3 deletions embed.fnc
Expand Up @@ -4412,7 +4412,6 @@ S |const char *|get_LC_ALL_display
S |bool |bool_setlocale_2008_i \
|const unsigned int index \
|NN const char *new_locale \
|const recalc_lc_all_t recalc_LC_ALL \
|const line_t line
S |const char *|querylocale_2008_i \
|const unsigned int index
Expand All @@ -4426,8 +4425,7 @@ S |const char *|calculate_LC_ALL_string \
# else
S |void |update_PL_curlocales_i \
|const unsigned int index \
|NN const char *new_locale \
|recalc_lc_all_t recalc_LC_ALL
|NN const char *new_locale
# endif
# elif defined(USE_LOCALE_THREADS) && \
!defined(USE_THREAD_SAFE_LOCALE) && \
Expand Down
4 changes: 2 additions & 2 deletions embed.h
Expand Up @@ -1300,14 +1300,14 @@
# define get_LC_ALL_display() S_get_LC_ALL_display(aTHX)
# endif
# if defined(USE_POSIX_2008_LOCALE)
# define bool_setlocale_2008_i(a,b,c,d) S_bool_setlocale_2008_i(aTHX_ a,b,c,d)
# define bool_setlocale_2008_i(a,b,c) S_bool_setlocale_2008_i(aTHX_ a,b,c)
# define querylocale_2008_i(a) S_querylocale_2008_i(aTHX_ a)
# define setlocale_from_aggregate_LC_ALL(a,b) S_setlocale_from_aggregate_LC_ALL(aTHX_ a,b)
# define use_curlocale_scratch() S_use_curlocale_scratch(aTHX)
# if defined(USE_QUERYLOCALE)
# define calculate_LC_ALL_string(a) S_calculate_LC_ALL_string(aTHX_ a)
# else
# define update_PL_curlocales_i(a,b,c) S_update_PL_curlocales_i(aTHX_ a,b,c)
# define update_PL_curlocales_i(a,b) S_update_PL_curlocales_i(aTHX_ a,b)
# endif
# elif defined(USE_LOCALE_THREADS) && \
!defined(USE_THREAD_SAFE_LOCALE) && \
Expand Down
59 changes: 9 additions & 50 deletions locale.c
Expand Up @@ -1344,7 +1344,7 @@ S_querylocale_2008_i(pTHX_ const unsigned int index)
/*---------------------------------------------------------------------------*/

# define bool_setlocale_i(i, locale) \
bool_setlocale_2008_i(i, locale, YES_RECALC_LC_ALL, __LINE__)
bool_setlocale_2008_i(i, locale, __LINE__)
# define bool_setlocale_c(cat, locale) \
bool_setlocale_i(cat##_INDEX_, locale)
# define bool_setlocale_r(cat, locale) \
Expand All @@ -1355,8 +1355,7 @@ S_querylocale_2008_i(pTHX_ const unsigned int index)
STATIC void
S_update_PL_curlocales_i(pTHX_
const unsigned int index,
const char * new_locale,
recalc_lc_all_t recalc_LC_ALL)
const char * new_locale)
{
/* This is a helper function for bool_setlocale_2008_i(), mostly used to
* make that function easier to read. */
Expand All @@ -1379,15 +1378,9 @@ S_update_PL_curlocales_i(pTHX_
Safefree(PL_curlocales[index]);
PL_curlocales[index] = savepv(new_locale);

/* And also LC_ALL if the input says to, including if this is the final
* iteration of a loop updating all sub-categories */
if ( recalc_LC_ALL == YES_RECALC_LC_ALL
|| ( recalc_LC_ALL == RECALCULATE_LC_ALL_ON_FINAL_INTERATION
&& index == LC_ALL_INDEX_ - 1))
{
/* Invalidate LC_ALL */
Safefree(PL_curlocales[LC_ALL_INDEX_]);
PL_curlocales[LC_ALL_INDEX_]= savepv(calculate_LC_ALL_string(PL_curlocales));
}
PL_curlocales[LC_ALL_INDEX_] = NULL;
}
}

Expand Down Expand Up @@ -1424,7 +1417,7 @@ S_setlocale_from_aggregate_LC_ALL(pTHX_ const char * locale, const line_t line)
* all the individual categories to "C", and override the furnished
* ones below. FALSE => No need to recalculate LC_ALL, as this is a
* temporary state */
if (! bool_setlocale_2008_i(LC_ALL_INDEX_, "C", DONT_RECALC_LC_ALL, line)) {
if (! bool_setlocale_2008_i(LC_ALL_INDEX_, "C", line)) {
setlocale_failure_panic_c(LC_ALL, locale_on_entry, "C", __LINE__, line);
NOT_REACHED; /* NOTREACHED */
}
Expand Down Expand Up @@ -1484,13 +1477,12 @@ S_setlocale_from_aggregate_LC_ALL(pTHX_ const char * locale, const line_t line)

/* And do the change. Don't recalculate LC_ALL; we'll do it
* ourselves after the loop */
if (! bool_setlocale_2008_i(i, individ_locale,
DONT_RECALC_LC_ALL, line))
if (! bool_setlocale_2008_i(i, individ_locale, line))
{

/* But if we have to back out, do fix up LC_ALL */
if (! bool_setlocale_2008_i(LC_ALL_INDEX_, locale_on_entry,
YES_RECALC_LC_ALL, line))
line))
{
setlocale_failure_panic_i(i, individ_locale,
locale, __LINE__, line);
Expand Down Expand Up @@ -1537,7 +1529,6 @@ S_bool_setlocale_2008_i(pTHX_
const unsigned int index,

const char * new_locale, /* The locale to set the category to */
const recalc_lc_all_t recalc_LC_ALL, /* Explained below */
const line_t line /* Called from this line number */
)
{
Expand All @@ -1548,21 +1539,7 @@ S_bool_setlocale_2008_i(pTHX_
* thread; thus it is thread-safe. It does this by using the POSIX 2008
* locale functions to emulate the behavior of setlocale(). By doing this,
* most locale-sensitive functions become thread-safe. The exceptions are
* mostly those that return a pointer to static memory.
*
* This function may be called in a tight loop that iterates over all
* categories. Because LC_ALL is not a "real" category, but merely the sum
* of all the other ones, such loops don't include LC_ALL. On systems that
* have querylocale() or similar, the current LC_ALL value is immediately
* retrievable; on systems lacking that feature, we have to keep track of
* LC_ALL ourselves. We could do that on each iteration, only to throw it
* away on the next, but the calculation is more than a trivial amount of
* work. Instead, the 'recalc_LC_ALL' parameter is set to
* RECALCULATE_LC_ALL_ON_FINAL_INTERATION to only do the calculation once.
* This function calls itself recursively in such a loop.
*
* When not in such a loop, the parameter is set to the other enum values
* DONT_RECALC_LC_ALL or YES_RECALC_LC_ALL. */
* mostly those that return a pointer to static memory. */

int mask = category_masks[index];
const locale_t entry_obj = uselocale((locale_t) 0);
Expand Down Expand Up @@ -1592,22 +1569,6 @@ S_bool_setlocale_2008_i(pTHX_
"(%" LINE_Tf "): bool_setlocale_2008_i"
" no-op to change to what it already was\n",
line));

# ifdef USE_PL_CURLOCALES

/* On the final iteration of a loop that needs to recalculate LC_ALL, do
* so. If no iteration changed anything, LC_ALL also doesn't change,
* but khw believes the complexity needed to keep track of that isn't
* worth it. */
if (UNLIKELY( recalc_LC_ALL == RECALCULATE_LC_ALL_ON_FINAL_INTERATION
&& index == LC_ALL_INDEX_ - 1))
{
Safefree(PL_curlocales[LC_ALL_INDEX_]);
PL_curlocales[LC_ALL_INDEX_] = savepv(calculate_LC_ALL_string(PL_curlocales));
}

# endif

return true;
}

Expand Down Expand Up @@ -1782,11 +1743,9 @@ S_bool_setlocale_2008_i(pTHX_
new_locale = querylocale_i(index);
}

PERL_UNUSED_ARG(recalc_LC_ALL);

# else

update_PL_curlocales_i(index, new_locale, recalc_LC_ALL);
update_PL_curlocales_i(index, new_locale);

# endif
# ifdef HAS_GLIBC_LC_MESSAGES_BUG
Expand Down
14 changes: 2 additions & 12 deletions perl.h
Expand Up @@ -1350,18 +1350,8 @@ typedef enum {

#ifdef PERL_CORE

/* Both typedefs are used in locale.c only, but defined here so that embed.fnc
* can generate the proper prototypes. */

typedef enum {
DONT_RECALC_LC_ALL,
YES_RECALC_LC_ALL,

/* Used in tight loops through all sub-categories, where LC_ALL won't be
* fully known until all subcategories are handled. */
RECALCULATE_LC_ALL_ON_FINAL_INTERATION
} recalc_lc_all_t;

/* These typedefs are used in locale.c only (and documented there), but defined
* here so that embed.fnc can generate the proper prototypes. */

typedef enum { /* Is the locale UTF8? */
LOCALE_NOT_UTF8,
Expand Down
4 changes: 2 additions & 2 deletions proto.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 25b59bf

Please sign in to comment.