amazon-ion / ion-python

A Python implementation of Amazon Ion.
https://amazon-ion.github.io/ion-docs/
Apache License 2.0
253 stars 50 forks source link

Remove PyObject_HasAttrString condition check. #319

Closed linlin-s closed 6 months ago

linlin-s commented 6 months ago

Issue #, if available: N/A

Description of changes: In this PR, we have eliminated the PyObject_HasAttrString condition check because the PyObject_GetAttrString function itself checks whether the attribute exists and will return NULL if it doesn't. If the PyObject_GetAttrString returns NULL, this indicates that an error has occurred, and we will handle the AttributeError under this condition.

Here is the benchmark results from benchmarking ion-python (C extension enabled) using shorthand log data. There is 21.23% performance improvement (time_mean) on shorthand log data and 3.35% performance improvement(time_mean) on single nested test data.

ion-python-benchmark-cli usage pattern to reproduce the benchmark results: python ion_benchmark_cli.py write --iterations 30 --warmups 10 --io-type file test_data.ion --format ion_binary

shorthand log file_size(B) time_min(ns) time_mean(ns) memory_usage_peak(B)
Before 21270622 3739596533.40 3798220835.84 42668611
After 21270622 2950278533.40 2991548144.17 42616337
single nested file_size(B) time_min(ns) time_mean(ns) memory_usage_peak(B)
Before 795 197943.52 204293.10 35887
After 795 193288.23 197446.85 29246

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

linlin-s commented 6 months ago

Here are the benchmark results after commit 4f4788d There is 20.62% performance improvement (timemean) on shorthand log data and 3.48%_ performance improvement(time_mean) on single nested test data.

shorthand log file_size(B) time_min(ns) time_mean(ns) memory_usage_peak(B)
Before 21270622 3745274325.00 3781944997.77 42696241
After 21270622 2960032208.40 3001857282.51 42644049
single nested name file_size(B) time_min(ns) time_mean(ns) memory_usage_peak(B)
Before 795 203792.17 209076.32 35595
After 795 193143.60 201790.09 29303