Skip to content

Commit

Permalink
Reduce scope of locking
Browse files Browse the repository at this point in the history
  • Loading branch information
daviddrysdale committed Apr 1, 2021
1 parent 439598d commit fca72cc
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 46 deletions.
11 changes: 0 additions & 11 deletions README.md
Expand Up @@ -210,17 +210,6 @@ load of metadata will not cause a pause or memory exhaustion):
The `phonenumberslite` version of the package does not include the geocoding, carrier and timezone metadata,
which can be useful if you have problems installing the main `phonenumbers` package due to space/memory limitations.

Thread Safety
-------------

The on-demand metadata loading described in the previous section means that synchronization is needed to allow
for the library being used in a multi-threaded environment. This synchronization (and its overheads) can be avoided by
either:

* calling `PhoneMetadata.load_all()` to load all metadata in advance
* setting `phonenumbers.phonemetadata.THREAD_SAFE_METADATA` to `False` (in a single-threaded
environment).

Project Layout
--------------

Expand Down
10 changes: 0 additions & 10 deletions python/HISTORY.md
Expand Up @@ -12,16 +12,6 @@ This file does not generally include descriptions of patch releases (vX.Y.Z
changes. (Metadata updates are best checked
[upstream](https://github.com/google/libphonenumber/blob/master/release_notes.txt).)

What's new in 8.12.??
---------------------

On-demand metadata loading is now protected by a mutex to allow use in a multi-threaded
environment. This synchronization (and its overheads) can be avoided by either:

- calling `PhoneMetadata.load_all()` to load all metadata in advance
- setting `phonenumbers.phonemetadata.THREAD_SAFE_METADATA` to `False` (in a single-threaded
environment).

What's new in 8.12.0
---------------------

Expand Down
31 changes: 6 additions & 25 deletions python/phonenumbers/phonemetadata.py
Expand Up @@ -22,18 +22,6 @@

REGION_CODE_FOR_NON_GEO_ENTITY = u("001")

# This global flag indicates whether on-demand loading of metadata should be
# done in a thread-safe (mutex-protected) manner.
#
# - Setting to True (the default) is safer but slower.
# - Setting to False avoids the mutex overhead, but means that metadata
# loading is not thread-safe. This should only be used if the application
# is single threaded.
#
# Calling PhoneMetadata.load_all() also avoids the mutex overhead, at the
# cost of increasing startup time and occupancy.
THREAD_SAFE_METADATA = True


class NumberFormat(UnicodeMixin, ImmutableMixin):
"""Representation of way that a phone number can be formatted for output"""
Expand Down Expand Up @@ -262,6 +250,9 @@ class PhoneMetadata(UnicodeMixin, ImmutableMixin):
to warn about breaking changes.
"""
# Lock that protects the *_available fields while they are being modified.
# The modificiation involves loading data from a file, so we cannot just
# rely on the GIL.
_metadata_lock = threading.Lock()
# If a region code is a key in this dict, metadata for that region is available.
# The corresponding value of the map is either:
Expand All @@ -283,40 +274,34 @@ class PhoneMetadata(UnicodeMixin, ImmutableMixin):

@classmethod
def metadata_for_region(kls, region_code, default=None):
if THREAD_SAFE_METADATA:
kls._metadata_lock.acquire()
loader = kls._region_available.get(region_code, None)
if loader is not None:
# Region metadata is available but has not yet been loaded. Do so now.
kls._metadata_lock.acquire()
loader(region_code)
kls._region_available[region_code] = None
if THREAD_SAFE_METADATA:
kls._metadata_lock.release()
return kls._region_metadata.get(region_code, default)

@classmethod
def short_metadata_for_region(kls, region_code, default=None):
if THREAD_SAFE_METADATA:
kls._metadata_lock.acquire()
loader = kls._short_region_available.get(region_code, None)
if loader is not None:
# Region short number metadata is available but has not yet been loaded. Do so now.
kls._metadata_lock.acquire()
loader(region_code)
kls._short_region_available[region_code] = None
if THREAD_SAFE_METADATA:
kls._metadata_lock.release()
return kls._short_region_metadata.get(region_code, default)

@classmethod
def metadata_for_nongeo_region(kls, country_code, default=None):
if THREAD_SAFE_METADATA:
kls._metadata_lock.acquire()
loader = kls._country_code_available.get(country_code, None)
if loader is not None:
# Region metadata is available but has not yet been loaded. Do so now.
kls._metadata_lock.acquire()
loader(country_code)
kls._country_code_available[country_code] = None
if THREAD_SAFE_METADATA:
kls._metadata_lock.release()
return kls._country_code_metadata.get(country_code, default)

Expand Down Expand Up @@ -351,10 +336,6 @@ def load_all(kls):
if loader is not None:
loader(country_code)
kls._country_code_available[region_code] = None
# There will be no more writes to the metadata, so no longer any need to
# do synchronization.
global THREAD_SAFE_METADATA
THREAD_SAFE_METADATA = False

@mutating_method
def __init__(self,
Expand Down

0 comments on commit fca72cc

Please sign in to comment.