Skip to content
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

Refactor Object getter/property macros to remove duplications #15383

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ysbaddaden
Copy link
Contributor

Extracts def macros to avoid most of the duplication between getter, getter?, property and property? as well as getter! and property! (be they for ivars or class vars).

The def macros are defined in the Crystal namespace to avoid polluting the Object type with internal, undocumented, macros.

It's still a bit of macro mess, but at least each implementation is no longer duplicated.

The downside is that macros generating calls to other macros is slowing the semantic passes and uses more memory. For example compiling crystal itself:

Before:

Semantic (top level):              00:00:00.366039601 ( 190.06MB)
Semantic (new):                    00:00:00.002077768 ( 190.06MB)
Semantic (type declarations):      00:00:00.033126181 ( 198.06MB)
Semantic (abstract def check):     00:00:00.015921465 ( 222.06MB)
Semantic (restrictions augmenter): 00:00:00.010128594 ( 222.06MB)
Semantic (ivars initializers):     00:00:02.880108440 (1152.00MB)
Semantic (cvars initializers):     00:00:00.007371344 (1160.00MB)

After:

Semantic (top level):              00:00:00.416716450 ( 222.14MB)
Semantic (new):                    00:00:00.002005603 ( 222.14MB)
Semantic (type declarations):      00:00:00.031955767 ( 230.14MB)
Semantic (abstract def check):     00:00:00.013552446 ( 254.14MB)
Semantic (restrictions augmenter): 00:00:00.009634944 ( 254.14MB)
Semantic (ivars initializers):     00:00:03.001631343 (1144.07MB)
Semantic (cvars initializers):     00:00:00.006819680 (1144.07MB)

Extracts def macros to avoid most of the duplication between getter,
getter?, property and property? and getter! and property! (be they for
ivars or class vars).

The def macros are defined in the `Crystal` namespace to avoid polluting
the `Object` type with internal, undocumented, macros.
@ysbaddaden
Copy link
Contributor Author

I simplified how I pass the block to the inner def_ macros, which requires a bit less memory.

make .build/crystal:

  • Semantic (new): 0.38s -> 0.42s (+10%) (190MB -> 214MB)
  • Semantic (ivars): 2.88s -> 2.96s (+3%) (1152MB -> 1104MB)

make .build/std_spec:

  • Semantic (new): 5.33s -> 5.43s (+2%) (622MB -> 630MB)
  • Semantic (ivars): 0.32s -> 0.05s (-84%) (654MB -> 710MB)

Weirdly, processing the ivars while building the std specs becomes magnitudes faster while using a bit more memory, while building crystal became a little slower but uses less memory.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant