Closed kurtbrose closed 1 year ago
Merging #235 (97e8481) into master (e677614) will decrease coverage by
0.01%
. The diff coverage is100.00%
.
@@ Coverage Diff @@
## master #235 +/- ##
==========================================
- Coverage 98.01% 98.01% -0.01%
==========================================
Files 27 27
Lines 4336 4335 -1
Branches 748 748
==========================================
- Hits 4250 4249 -1
Misses 56 56
Partials 30 30
Impacted Files | Coverage Δ | |
---|---|---|
glom/core.py | 98.04% <100.00%> (-0.01%) |
:arrow_down: |
:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more
If memory + a quick glance serves, perf_report
generates a bit of an extreme case, no? I'm guessing there's no regression, but there's also not much expectation of speedup in a regular case? Also, any expectation of memory usage changes?
Sidenote: Can you make perf_report spit out the python runtime info too? (version, etc.)
Attribute access via T is very common -- every string in Auto mode -- and weakref is significantly slower than regular attribute access.
The test is meant to be representative; it's based on serialization -- a 4 attribute dict in Auto mode
On Mon, Feb 7, 2022, 10:52 PM Mahmoud Hashemi @.***> wrote:
If memory + a quick glance serves, perf_report generates a bit of an extreme case, no? I'm guessing there's no regression, but there's also not much expectation of speedup in a regular case? Also, any expectation of memory usage changes?
Sidenote: Can you make perf_report spit out the python runtime info too? (version, etc.)
— Reply to this email directly, view it on GitHub https://github.com/mahmoud/glom/pull/235#issuecomment-1032269114, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEEZPVZJCTR2IRLRDOYBY3U2C4UBANCNFSM5NZQCGOA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
You are receiving this because you authored the thread.Message ID: @.***>
I think cython is probably the long term future for glom. Right off the bat that might save us 30% just from the byte code dispatch.
(My next experiment is going to be cython without touching the code to see.)
On Mon, Feb 7, 2022, 11:02 PM Kurt Rose @.***> wrote:
Attribute access via T is very common -- every string in Auto mode -- and weakref is significantly slower than regular attribute access.
The test is meant to be representative; it's based on serialization -- a 4 attribute dict in Auto mode
On Mon, Feb 7, 2022, 10:52 PM Mahmoud Hashemi @.***> wrote:
If memory + a quick glance serves, perf_report generates a bit of an extreme case, no? I'm guessing there's no regression, but there's also not much expectation of speedup in a regular case? Also, any expectation of memory usage changes?
Sidenote: Can you make perf_report spit out the python runtime info too? (version, etc.)
— Reply to this email directly, view it on GitHub https://github.com/mahmoud/glom/pull/235#issuecomment-1032269114, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEEZPVZJCTR2IRLRDOYBY3U2C4UBANCNFSM5NZQCGOA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
You are receiving this because you authored the thread.Message ID: @.***>
That's true, I just rereviewed perf_report and while there are thousands of generated items, they're all pretty ordinary. (Still wondering if we ought to take a look around and see if there are any open-source glom use cases that are even more realistic.)
To the other questions:
Just to paper trail here -- the test was on Python 3.9. Memory usage I'd expect to be slightly better, but I'm not concerned about the size of the spec in memory so much. Presumably it's much smaller than the data.
was just blowing the dust off perf stuff; I think this is mergable.
We may even be able to go a step further -- once T is a "normal" object with it's internal state in __ops__
, it probably doesn't need to override __getstate__
and __setstate__
any more, which is a nice reduction in code complexity.
this is a more conservative simplifying of the guts of T -- rather than making changes to the internal structure, this just moves the internal tuple from a weakref to a dunder attribute of the T object
specifically of interest is the perf difference -- slight but consistent; my "standard spec" perf test gets about 1 microsecond faster -- taking it from 85x as slow as straight python to 80x as slow
I think there are also benefits to simplifying the code; but that requires a bit more than this quick replace of
_T_PATHS[t]
witht.__ops__
to be fully realized