perf: optimize timestamp parsing in metadata processing - #1045
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the _parse_timestamp method in gcsfs/core.py to be a static method and simplifies its implementation using datetime.fromisoformat. It also adds comprehensive unit tests to verify various timestamp formats. However, the simplified implementation introduces a compatibility issue on Python 3.10, where datetime.fromisoformat does not support arbitrary fractional second lengths (such as 1, 2, 4, or 5 digits), leading to potential runtime crashes and test failures. A conditional fallback based on the Python version is recommended to maintain compatibility.
| @staticmethod | ||
| def _parse_timestamp(timestamp): | ||
| return datetime.fromisoformat(timestamp.replace("Z", "+00:00")) |
There was a problem hiding this comment.
In Python 3.10, datetime.fromisoformat does not support arbitrary fractional second lengths (it only supports exactly 0, 3, or 6 digits). If a timestamp has a different number of fractional digits (such as 1, 2, 4, or 5 digits, which are explicitly covered in the new test cases like 2024-01-15T10:30:00.12Z), fromisoformat will raise a ValueError on Python 3.10.
Since gcsfs supports Python >= 3.10, this change will cause test failures and runtime crashes for Python 3.10 users.
To resolve this while maintaining maximum performance:
- On Python 3.11+, we can use the native
datetime.fromisoformatdirectly (which natively supports theZsuffix and arbitrary fractional seconds, making it even faster as it avoids string replacement). - On Python 3.10, we can use a fast compatibility fallback that only pads the fractional seconds when their length is not 3 or 6.
We can achieve this with zero runtime overhead by conditionally defining the @staticmethod at class definition time using sys.version_info.
if sys.version_info >= (3, 11):
@staticmethod
def _parse_timestamp(timestamp):
return datetime.fromisoformat(timestamp)
else:
@staticmethod
def _parse_timestamp(timestamp):
if timestamp.endswith('Z'):
timestamp = timestamp[:-1]
if '.' in timestamp:
frac_len = len(timestamp.rsplit('.', 1)[-1])
if frac_len not in (3, 6):
timestamp = timestamp + '0' * (6 - frac_len)
return datetime.fromisoformat(timestamp + '+00:00')
return datetime.fromisoformat(timestamp)
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1045 +/- ##
==========================================
+ Coverage 90.10% 90.18% +0.07%
==========================================
Files 16 16
Lines 3679 3739 +60
==========================================
+ Hits 3315 3372 +57
- Misses 364 367 +3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Description:
Optimizes
GCSFileSystem._parse_timestampby replacing legacy string slicing and zero-padding logic with@staticmethodanddatetime.fromisoformat(timestamp.replace("Z", "+00:00")).This yields a ~2.2x speedup on timestamp parsing and reduces total
_process_objectlatency by ~33%.Rationale & Backward Compatibility
rsplitlogic was originally added as a workaround for older Python versions (<3.10) wherefromisoformat()was restrictive about fractional-second padding.gcsfsrequirespython >= 3.10, standard GCS RFC 3339 timestamps (milliseconds.000Z, microseconds.ffffffZ, and whole secondsZ) are natively supported byfromisoformat()when replacing"Z"with"+00:00".datetime.fromisoformat()SpecificationBenchmarks (
timeit, 500k iterations)_parse_timestamp(milliseconds.000Z)_parse_timestamp(microseconds.123456Z)_parse_timestamp(whole secondsZ)_process_object(2 timestamps)Testing
test_parse_timestamp_formatsandtest_process_object_timestampstogcsfs/tests/test_core.pycovering all GCS timestamp formats and precisions.