Skip to content

Improve generation of typealiases #233

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

Merged
merged 2 commits into from
Dec 19, 2020
Merged

Conversation

kimikage
Copy link
Collaborator

@kimikage kimikage commented Sep 23, 2020

This is yet another solution for the issue which is going to be fixed by PR #232.

This also includes the preparation for #228. I don't believe typechar is used in other packages, but I've added depwarn because there are cases where showtype is used.

Edit:
This PR is not intended to replace #232, but to demonstrate a different approach. However, in any case, I believe that some fixes will be required for #228.

@codecov
Copy link

codecov bot commented Sep 23, 2020

Codecov Report

Merging #233 (953001e) into master (fe2135d) will decrease coverage by 0.11%.
The diff coverage is 94.11%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #233      +/-   ##
==========================================
- Coverage   95.68%   95.57%   -0.12%     
==========================================
  Files           6        6              
  Lines         742      746       +4     
==========================================
+ Hits          710      713       +3     
- Misses         32       33       +1     
Impacted Files Coverage Δ
src/FixedPointNumbers.jl 95.43% <87.50%> (-0.32%) ⬇️
src/deprecations.jl 50.00% <100.00%> (+50.00%) ⬆️
src/fixed.jl 96.89% <100.00%> (ø)
src/normed.jl 95.47% <100.00%> (ø)

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 fe2135d...953001e. Read the comment docs.

@kimikage kimikage marked this pull request as draft September 23, 2020 15:02
@kimikage kimikage force-pushed the alias branch 2 times, most recently from aaf1396 to c3304ef Compare September 24, 2020 12:25
@kimikage kimikage marked this pull request as ready for review September 24, 2020 12:26
@kimikage
Copy link
Collaborator Author

I am going to try to see if the symbol-generating "macro" is effective.

@kimikage
Copy link
Collaborator Author

I've tried several metaprogramming techniques and generated function seems to be the simplest.

julia> versioninfo()
Julia Version 1.6.0-DEV.1050
Commit fd57021b33 (2020-09-25 18:40 UTC)
Platform Info:
  OS: Windows (x86_64-w64-mingw32)
  CPU: Intel(R) Core(TM) i7-8565U CPU @ 1.80GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-10.0.1 (ORCJIT, skylake)

julia> @time using FixedPointNumbers
[ Info: Precompiling FixedPointNumbers [53c48c17-4a7d-5ca2-90c5-79b7896eea93]
  2.279809 seconds (213.42 k allocations: 13.772 MiB, 0.25% gc time) # master
  1.179185 seconds (209.43 k allocations: 13.267 MiB)                # this PR

printing ARGB32(cf. #232 (comment))

julia> versioninfo()
Julia Version 1.5.2
Commit 539f3ce943 (2020-09-23 23:17 UTC)
Platform Info:
  OS: Windows (x86_64-w64-mingw32)
  CPU: Intel(R) Core(TM) i7-8565U CPU @ 1.80GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-9.0.1 (ORCJIT, skylake)

julia> @benchmark dump(io, $img) # master
BenchmarkTools.Trial:
  memory estimate:  1.70 MiB
  allocs estimate:  9216
  --------------
  minimum time:     740.600 μs (0.00% GC)
  median time:      1.450 ms (0.00% GC)
  mean time:        1.666 ms (16.23% GC)
  maximum time:     181.050 ms (0.46% GC)
  --------------
  samples:          3000
  evals/sample:     1

julia> @benchmark dump(io, $img) # this PR
BenchmarkTools.Trial:
  memory estimate:  1.70 MiB
  allocs estimate:  9216
  --------------
  minimum time:     622.900 μs (0.00% GC)
  median time:      1.344 ms (0.00% GC)
  mean time:        1.562 ms (17.51% GC)
  maximum time:     202.086 ms (8.59% GC)
  --------------
  samples:          3211
  evals/sample:     1

The original reason for submitting this PR is never to speed up printing colors, but I am not in a position to negate the need for the specialization anymore.

I will modify this PR as you asked and will not merge this PR unless there is a consensus.

@kimikage
Copy link
Collaborator Author

The essence of this PR is to reduce the precompile time and to support two-character type prefixes.
I don't think it's selfish to merge this, as this PR has been open for over two months...

@kimikage kimikage merged commit 39e7d9b into JuliaMath:master Dec 19, 2020
@kimikage kimikage deleted the alias branch December 19, 2020 04:53
@kimikage kimikage mentioned this pull request Apr 30, 2024
38 tasks
kimikage added a commit to kimikage/FixedPointNumbers.jl that referenced this pull request Apr 30, 2024
kimikage added a commit to kimikage/FixedPointNumbers.jl that referenced this pull request Apr 30, 2024
kimikage added a commit to kimikage/FixedPointNumbers.jl that referenced this pull request May 1, 2024
kimikage added a commit to kimikage/FixedPointNumbers.jl that referenced this pull request May 1, 2024
kimikage added a commit to kimikage/FixedPointNumbers.jl that referenced this pull request May 1, 2024
kimikage added a commit to kimikage/FixedPointNumbers.jl that referenced this pull request May 1, 2024
kimikage added a commit to kimikage/FixedPointNumbers.jl that referenced this pull request May 1, 2024
kimikage added a commit to kimikage/FixedPointNumbers.jl that referenced this pull request May 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant