Remove usage of register keyword in Extension Modules #26264
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WillAyd
changed the title
gfyoung
approved these changes
May 2, 2019
jbrockmendel
reviewed
May 2, 2019
Codecov Report
@@ Coverage Diff @@
## master #26264 +/- ##
===========================================
- Coverage 91.98% 40.72% -51.26%
===========================================
Files 175 175
Lines 52386 52386
===========================================
- Hits 48185 21334 -26851
- Misses 4201 31052 +26851
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #26264 +/- ##
==========================================
+ Coverage 91.98% 91.98% +<.01%
==========================================
Files 175 175
Lines 52386 52379 -7
==========================================
- Hits 48185 48179 -6
+ Misses 4201 4200 -1
Continue to review full report at Codecov.
|
|
these were all just added here: https://github.com/pandas-dev/pandas/pull/25784/files |
jreback
requested changes
May 2, 2019
|
@WillAyd try running: |
|
Ran that and benchmarks still showed no change: $ asv continuous -f 1.05 7fafb356549f1c52a9896429ce0241487f42bea9 a491c2b217b69208e810c23ce6db7a21d033fd06 -b io.csv -a sample_time=2 -a warmup_time=2
· Creating environments
· Discovering benchmarks.
·· Uninstalling from conda-py3.6-Cython-matplotlib-numexpr-numpy-openpyxl-pytables-pytest-scipy-sqlalchemy-xlrd-xlsxwriter-xlwt.
·· Installing a491c2b2 <remove-register~2> into conda-py3.6-Cython-matplotlib-numexpr-numpy-openpyxl-pytables-pytest-scipy-sqlalchemy-xlrd-xlsxwriter-xlwt..
· Running 36 total benchmarks (2 commits * 1 environments * 18 benchmarks)
[ 0.00%] · For pandas commit 7fafb356 <master~2> (round 1/2):
[ 0.00%] ·· Building for conda-py3.6-Cython-matplotlib-numexpr-numpy-openpyxl-pytables-pytest-scipy-sqlalchemy-xlrd-xlsxwriter-xlwt...
[ 0.00%] ·· Benchmarking conda-py3.6-Cython-matplotlib-numexpr-numpy-openpyxl-pytables-pytest-scipy-sqlalchemy-xlrd-xlsxwriter-xlwt
[ 1.39%] ··· Running (io.csv.ReadCSVCategorical.time_convert_direct--)...
[ 5.56%] ··· Running (io.csv.ReadCSVDInferDatetimeFormat.time_read_csv--).
[ 6.94%] ··· Running (io.csv.ReadCSVFloatPrecision.time_read_csv--).
[ 8.33%] ··· Running (io.csv.ReadCSVFloatPrecision.time_read_csv_python_engine--).
[ 11.11%] ··· Running (io.csv.ReadCSVParseDates.time_baseline--)...
[ 15.28%] ··· Running (io.csv.ReadCSVSkipRows.time_skipprows--)..
[ 18.06%] ··· Running (io.csv.ReadUint64Integers.time_read_uint64--)...
[ 22.22%] ··· Running (io.csv.ToCSV.time_frame--).
[ 23.61%] ··· Running (io.csv.ToCSVDatetime.time_frame_date_formatting--)...
[ 25.00%] · For pandas commit a491c2b2 <remove-register~2> (round 1/2):
[ 25.00%] ·· Building for conda-py3.6-Cython-matplotlib-numexpr-numpy-openpyxl-pytables-pytest-scipy-sqlalchemy-xlrd-xlsxwriter-xlwt...
[ 25.00%] ·· Benchmarking conda-py3.6-Cython-matplotlib-numexpr-numpy-openpyxl-pytables-pytest-scipy-sqlalchemy-xlrd-xlsxwriter-xlwt
[ 26.39%] ··· Running (io.csv.ReadCSVCategorical.time_convert_direct--)...
[ 30.56%] ··· Running (io.csv.ReadCSVDInferDatetimeFormat.time_read_csv--).
[ 31.94%] ··· Running (io.csv.ReadCSVFloatPrecision.time_read_csv--).
[ 33.33%] ··· Running (io.csv.ReadCSVFloatPrecision.time_read_csv_python_engine--).
[ 36.11%] ··· Running (io.csv.ReadCSVParseDates.time_baseline--)...
[ 40.28%] ··· Running (io.csv.ReadCSVSkipRows.time_skipprows--)..
[ 43.06%] ··· Running (io.csv.ReadUint64Integers.time_read_uint64--)...
[ 47.22%] ··· Running (io.csv.ToCSV.time_frame--).
[ 48.61%] ··· Running (io.csv.ToCSVDatetime.time_frame_date_formatting--)...
[ 50.00%] · For pandas commit a491c2b2 <remove-register~2> (round 2/2):
[ 50.00%] ·· Benchmarking conda-py3.6-Cython-matplotlib-numexpr-numpy-openpyxl-pytables-pytest-scipy-sqlalchemy-xlrd-xlsxwriter-xlwt
[ 51.39%] ··· io.csv.ReadCSVCategorical.time_convert_direct 42.1±0.5ms
[ 52.78%] ··· io.csv.ReadCSVCategorical.time_convert_post 59.2±1ms
[ 54.17%] ··· io.csv.ReadCSVComment.time_comment 28.8±0.2ms
[ 55.56%] ··· io.csv.ReadCSVDInferDatetimeFormat.time_read_csv ok
[ 55.56%] ··· ======================= ============ ============= =============
-- format
----------------------- ----------------------------------------
infer_datetime_format custom iso8601 ymd
======================= ============ ============= =============
True 6.38±0.1ms 2.71±0.07ms 2.69±0.06ms
False 108±2ms 2.17±0.09ms 2.12±0.1ms
======================= ============ ============= =============
[ 56.94%] ··· io.csv.ReadCSVFloatPrecision.time_read_csv ok
[ 56.94%] ··· ===== ============= ============= ================ ============= ============= ================
-- decimal / float_precision
----- -----------------------------------------------------------------------------------------
sep . / None . / high . / round_trip _ / None _ / high _ / round_trip
===== ============= ============= ================ ============= ============= ================
, 2.01±0.07ms 1.89±0.08ms 2.92±0.1ms 2.32±0.1ms 2.35±0.06ms 2.34±0.06ms
; 2.22±0.04ms 2.07±0.04ms 2.94±0.1ms 2.29±0.05ms 2.28±0.07ms 2.20±0.03ms
===== ============= ============= ================ ============= ============= ================
[ 58.33%] ··· io.csv.ReadCSVFloatPrecision.time_read_csv_python_engine ok
[ 58.33%] ··· ===== ============= ============= ================ ============= ============= ================
-- decimal / float_precision
----- -----------------------------------------------------------------------------------------
sep . / None . / high . / round_trip _ / None _ / high _ / round_trip
===== ============= ============= ================ ============= ============= ================
, 4.51±0.07ms 4.61±0.08ms 4.57±0.05ms 3.99±0.03ms 4.08±0.07ms 4.02±0.1ms
; 4.56±0.1ms 4.70±0.06ms 4.62±0.04ms 4.12±0.05ms 4.07±0.02ms 4.01±0.1ms
===== ============= ============= ================ ============= ============= ================
[ 59.72%] ··· io.csv.ReadCSVMemoryGrowth.mem_parser_chunks 0
[ 61.11%] ··· io.csv.ReadCSVParseDates.time_baseline 2.40±0.03ms
[ 62.50%] ··· io.csv.ReadCSVParseDates.time_multiple_date 2.88±0.03ms
[ 63.89%] ··· io.csv.ReadCSVParseSpecialDate.time_read_special_date ok
[ 63.89%] ··· ======== ============
param1
-------- ------------
mY 52.3±0.6ms
mdY 21.8±0.4ms
hm 431±8ms
======== ============
[ 65.28%] ··· io.csv.ReadCSVSkipRows.time_skipprows ok
[ 65.28%] ··· ========== ============
skiprows
---------- ------------
None 20.3±0.9ms
10000 13.6±0.2ms
========== ============
[ 66.67%] ··· io.csv.ReadCSVThousands.time_thousands ok
[ 66.67%] ··· ===== ============ ============
-- thousands
----- -------------------------
sep None ,
===== ============ ============
, 16.2±0.1ms 17.2±0.7ms
| 16.1±0.2ms 17.7±0.5ms
===== ============ ============
[ 68.06%] ··· io.csv.ReadUint64Integers.time_read_uint64 3.83±0.1ms
[ 69.44%] ··· io.csv.ReadUint64Integers.time_read_uint64_na_values 7.24±0.3ms
[ 70.83%] ··· io.csv.ReadUint64Integers.time_read_uint64_neg_values 6.63±0.3ms
[ 72.22%] ··· io.csv.ToCSV.time_frame ok
[ 72.22%] ··· ======= ============
kind
------- ------------
wide 147±0.9ms
long 234±0.8ms
mixed 30.1±0.6ms
======= ============
[ 73.61%] ··· io.csv.ToCSVDatetime.time_frame_date_formatting 18.9±0.7ms
[ 75.00%] ··· io.csv.ToCSVDatetimeBig.time_frame ok
[ 75.00%] ··· ======== ============
obs
-------- ------------
1000 6.34±0.1ms
10000 54.3±0.7ms
100000 555±7ms
======== ============
.
[ 75.00%] · For pandas commit 7fafb356 <master~2> (round 2/2):
[ 75.00%] ·· Building for conda-py3.6-Cython-matplotlib-numexpr-numpy-openpyxl-pytables-pytest-scipy-sqlalchemy-xlrd-xlsxwriter-xlwt...
[ 75.00%] ·· Benchmarking conda-py3.6-Cython-matplotlib-numexpr-numpy-openpyxl-pytables-pytest-scipy-sqlalchemy-xlrd-xlsxwriter-xlwt
[ 76.39%] ··· io.csv.ReadCSVCategorical.time_convert_direct 44.2±0.6ms
[ 77.78%] ··· io.csv.ReadCSVCategorical.time_convert_post 60.7±2ms
[ 79.17%] ··· io.csv.ReadCSVComment.time_comment 29.0±0.8ms
[ 80.56%] ··· io.csv.ReadCSVDInferDatetimeFormat.time_read_csv ok
[ 80.56%] ··· ======================= ============ ============= =============
-- format
----------------------- ----------------------------------------
infer_datetime_format custom iso8601 ymd
======================= ============ ============= =============
True 6.63±0.1ms 2.65±0.01ms 2.74±0.03ms
False 109±3ms 2.22±0.2ms 2.01±0.2ms
======================= ============ ============= =============
[ 81.94%] ··· io.csv.ReadCSVFloatPrecision.time_read_csv ok
[ 81.94%] ··· ===== ============= ============= ================ ============ ============= ================
-- decimal / float_precision
----- ----------------------------------------------------------------------------------------
sep . / None . / high . / round_trip _ / None _ / high _ / round_trip
===== ============= ============= ================ ============ ============= ================
, 1.95±0.03ms 1.91±0.03ms 2.76±0.1ms 2.21±0.1ms 2.13±0.06ms 2.32±0.07ms
; 2.06±0.01ms 2.03±0.04ms 2.82±0.03ms 2.19±0.1ms 2.12±0.3ms 2.16±0.09ms
===== ============= ============= ================ ============ ============= ================
[ 83.33%] ··· io.csv.ReadCSVFloatPrecision.time_read_csv_python_engine ok
[ 83.33%] ··· ===== ============= ============= ================ ============= ============= ================
-- decimal / float_precision
----- -----------------------------------------------------------------------------------------
sep . / None . / high . / round_trip _ / None _ / high _ / round_trip
===== ============= ============= ================ ============= ============= ================
, 4.34±0.2ms 4.57±0.1ms 4.53±0.08ms 4.03±0.05ms 4.21±0.1ms 4.02±0.1ms
; 4.64±0.08ms 4.72±0.05ms 4.60±0.08ms 4.11±0.09ms 4.13±0.05ms 3.94±0.05ms
===== ============= ============= ================ ============= ============= ================
[ 84.72%] ··· io.csv.ReadCSVMemoryGrowth.mem_parser_chunks 0
[ 86.11%] ··· io.csv.ReadCSVParseDates.time_baseline 2.34±0.02ms
[ 87.50%] ··· io.csv.ReadCSVParseDates.time_multiple_date 2.92±0.05ms
[ 88.89%] ··· io.csv.ReadCSVParseSpecialDate.time_read_special_date ok
[ 88.89%] ··· ======== ============
param1
-------- ------------
mY 52.7±0.4ms
mdY 21.7±0.2ms
hm 418±8ms
======== ============
[ 90.28%] ··· io.csv.ReadCSVSkipRows.time_skipprows ok
[ 90.28%] ··· ========== ============
skiprows
---------- ------------
None 19.4±0.4ms
10000 13.3±0.2ms
========== ============
[ 91.67%] ··· io.csv.ReadCSVThousands.time_thousands ok
[ 91.67%] ··· ===== ============ ============
-- thousands
----- -------------------------
sep None ,
===== ============ ============
, 16.6±0.7ms 17.4±0.5ms
| 16.2±0.1ms 17.6±0.4ms
===== ============ ============
[ 93.06%] ··· io.csv.ReadUint64Integers.time_read_uint64 3.64±0.05ms
[ 94.44%] ··· io.csv.ReadUint64Integers.time_read_uint64_na_values 6.75±0.4ms
[ 95.83%] ··· io.csv.ReadUint64Integers.time_read_uint64_neg_values 6.44±0.1ms
[ 97.22%] ··· io.csv.ToCSV.time_frame ok
[ 97.22%] ··· ======= ============
kind
------- ------------
wide 145±4ms
long 225±4ms
mixed 29.9±0.3ms
======= ============
[ 98.61%] ··· io.csv.ToCSVDatetime.time_frame_date_formatting 18.3±0.3ms
[100.00%] ··· io.csv.ToCSVDatetimeBig.time_frame ok
[100.00%] ··· ======== =============
obs
-------- -------------
1000 6.25±0.09ms
10000 54.1±0.5ms
100000 548±2ms
======== =============
.
BENCHMARKS NOT SIGNIFICANTLY CHANGED.FWIW gcc at least also discourages the use of the register keyword for local variables: https://gcc.gnu.org/onlinedocs/gcc/Local-Register-Variables.html#Local-Register-Variables |
|
I ran a test with |
jreback
reviewed
May 3, 2019
doc/source/whatsnew/v0.25.0.rst
Outdated
| @@ -427,6 +427,7 @@ Other | |||
| ^^^^^ | |||
|
|
|||
| - Removed unused C functions from vendored UltraJSON implementation (:issue:`26198`) | |||
| - Removed usage of ``register`` keyword in C extension modules (:issue:`26263`) | |||
There was a problem hiding this comment.
not necessary as the introduction was in 0.25 (u can add this issue number to the other note if u want)
jreback
approved these changes
May 5, 2019
|
thanks @WillAyd |
My guess is it was relevant for Windows + Python 2 (this combo used VS2008 to build), more modern compilers might be caching stuff to registers more eagerly. That said, @WillAyd did you run some benchmarks on Windows? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
git diff upstream/master -u -- "*.py" | flake8 --diffASV results show no difference:
As I am fairly certain this keyword was being entirely ignored by the compilers anyway (see issue for reasoning)