8000 py/obj.h: Add m_new_obj_with_type macro. by jimmo · Pull Request #8576 · micropython/micropython · GitHub
[go: up one dir, main page]

Skip to content

py/obj.h: Add m_new_obj_with_type macro. #8576

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from

Conversation

jimmo
Copy link
Member
@jimmo jimmo commented Apr 22, 2022

This is to replace the following:

mp_foo_obj_t *self = m_new_obj(mp_foo_obj_t);
self->base.type = &mp_type_foo;

with:

mp_foo_obj_t *self = m_new_obj_with_type(mp_foo_obj_t, &mp_type_foo);

Calling the function is less code than inlining setting the type everywhere, adds up to 100 bytes on PYBV11. Small, but every byte counts, right? :p

It also helps to avoid an easy mistake of forgetting to set the type.

Perf impact is negligible, possibly even improved? I specifically excluded mp_obj_new_float from using this (otherwise there was about a 1% decrease across the fp-heavy tests).

$ ./run-perfbench.py -s ~/obj-type-pybv11-baseline ~/obj-type-pybv11-helper-float 
diff of scores (higher is better)
N=100 M=100                /home/jimmo/obj-type-pybv11-baseline -> /home/jimmo/obj-type-pybv11-helper-float         diff      diff% (error%)
bm_chaos.py                    358.07 ->     366.90 :      +8.83 =  +2.466% (+/-0.01%)
bm_fannkuch.py                  76.62 ->      77.21 :      +0.59 =  +0.770% (+/-0.01%)
bm_fft.py                     2542.13 ->    2527.94 :     -14.19 =  -0.558% (+/-0.00%)
bm_float.py                   5916.09 ->    6066.47 :    +150.38 =  +2.542% (+/-0.02%)
bm_hexiom.py                    43.71 ->      43.80 :      +0.09 =  +0.206% (+/-0.00%)
bm_nqueens.py                 4398.49 ->    4363.18 :     -35.31 =  -0.803% (+/-0.00%)
bm_pidigits.py                 640.04 ->     637.61 :      -2.43 =  -0.380% (+/-0.22%)
misc_aes.py                    412.97 ->     421.05 :      +8.08 =  +1.957% (+/-0.01%)
misc_mandel.py              
8000
  3520.42 ->    3521.73 :      +1.31 =  +0.037% (+/-0.00%)
misc_pystone.py               2275.25 ->    2302.60 :     +27.35 =  +1.202% (+/-0.00%)
misc_raytrace.py               376.95 ->     386.30 :      +9.35 =  +2.480% (+/-0.01%)

@dpgeorge dpgeorge added the py-core Relates to py/ directory in source label Apr 29, 2022
@jimmo jimmo force-pushed the new-obj-type branch 2 times, most recently from 450e986 to 1114a6c Compare April 29, 2022 02:09
@codecov-commenter
Copy link

Codecov Report

Merging #8576 (1114a6c) into master (44186ef) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #8576      +/-   ##
==========================================
- Coverage   98.23%   98.22%   -0.01%     
==========================================
  Files         154      154              
  Lines       20364    20311      -53     
==========================================
- Hits        20004    19951      -53     
  Misses        360      360              
Impacted Files Coverage Δ
py/obj.h 100.00% <ø> (ø)
extmod/machine_signal.c 92.10% <100.00%> (-0.21%) ⬇️
extmod/modbtree.c 96.89% <100.00%> (-0.02%) ⬇️
extmod/modframebuf.c 100.00% <100.00%> (ø)
extmod/moduasyncio.c 99.20% <100.00%> (-0.01%) ⬇️
extmod/moducryptolib.c 92.15% <100.00%> (-0.08%) ⬇️
extmod/moductypes.c 97.89% <100.00%> (-0.04%) ⬇️
extmod/moduhashlib.c 100.00% <100.00%> (ø)
extmod/modure.c 100.00% <100.00%> (ø)
extmod/modutimeq.c 100.00% <100.00%> (ø)
... and 37 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 44186ef...1114a6c. Read the comment docs.

@dpgeorge
Copy link
Member

please rebase on latest master to pick up new renesas-ra port, which has 7 uses of m_new_obj()

@jimmo
Copy link
Member Author
jimmo commented Apr 29, 2022

Done. Not all 7 were suitable (two in timer.c immediately memset, and in mpthreadport.c it isn't actually an object).

@dpgeorge
Copy link
Member

The first commit includes changes to py/objfloat.c and py/objset.c which I think should be in the second commit (and the set change is wrong, uses an old version of the marco).

In the second commit some cc3200 HAL files are unnecessarily changed (maybe line endings?).

jimmo added 3 commits May 3, 2022 21:18
This is to replace the following:

  mp_foo_obj_t *self = m_new_obj(mp_foo_obj_t);
  self->base.type = &mp_type_foo;

with:

  mp_foo_obj_t *self = mp_obj_malloc(mp_foo_obj_t, &mp_type_foo);

Calling the function is less code than inlining setting the type
everywhere, adds up to ~100 bytes on PYBV11.

It also helps to avoid an easy mistake of forgetting to set the type.

Signed-off-by: Jim Mussared <jim.mussared@gmail.com>
Signed-off-by: Jim Mussared <jim.mussared@gmail.com>
8000
This replaces occurences of
  foo_t *foo = m_new_obj(foo_t);
  foo->base.type = &foo_type;
with
  foo_t *foo = mp_obj_malloc(foo_t, &foo_type);

Excludes any places where base is a sub-field or when new0/memset is used.

Signed-off-by: Jim Mussared <jim.mussared@gmail.com>
@jimmo
Copy link
Member Author
jimmo commented May 3, 2022

Done.

@dpgeorge
Copy link
Member
dpgeorge commented May 3, 2022

Thanks for fixing. Merged in 709e832 through 0e7bfc8

@dpgeorge dpgeorge closed this May 3, 2022
iabdalkader added a commit to iabdalkader/micropython that referenced this pull request Jun 9, 2022
* Changes introduced in micropython#8576 missed changing this call to mp_obj_malloc.
dlech added a commit to dlech/micropython that referenced this pull request May 7, 2023
This makes use of mp_obj_malloc() in more places to reduce binary size.

Also see micropython#8576.

Signed-off-by: David Lechner <david@pybricks.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
py-core Relates to py/ directory in source
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0