Skip to content

Conversation

KristofferC
Copy link
Member

@KristofferC KristofferC commented Jul 28, 2019

This avoids creating all the tiny Frame objects over and over.

Running the tight loop benchmark:

# Master
julia> @btime @interpret f(X)
  215.089 ms (1979122 allocations: 64.04 MiB)

# PR
julia> @btime @interpret f(X)
  207.504 ms (1749055 allocations: 49.24 MiB)

The time difference is not that big but the allocations are only 75% and it is possible this will start to matter more when other things are more optimized.

@codecov-io
Copy link

Codecov Report

Merging #308 into master will increase coverage by 0.33%.
The diff coverage is 98.83%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #308      +/-   ##
=========================================
+ Coverage   88.17%   88.5%   +0.33%     
=========================================
  Files          11      11              
  Lines        1742    1766      +24     
=========================================
+ Hits         1536    1563      +27     
+ Misses        206     203       -3
Impacted Files Coverage Δ
src/types.jl 74.77% <100%> (+3.34%) ⬆️
src/breakpoints.jl 92.64% <100%> (+3.67%) ⬆️
src/interpret.jl 86.31% <100%> (ø) ⬆️
src/utils.jl 85.57% <100%> (+0.35%) ⬆️
src/construct.jl 91.9% <97.56%> (-0.5%) ⬇️

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 f9ebc94...0595137. Read the comment docs.

1 similar comment
@codecov-io
Copy link

codecov-io commented Jul 28, 2019

Codecov Report

Merging #308 into master will increase coverage by 0.33%.
The diff coverage is 98.83%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #308      +/-   ##
=========================================
+ Coverage   88.17%   88.5%   +0.33%     
=========================================
  Files          11      11              
  Lines        1742    1766      +24     
=========================================
+ Hits         1536    1563      +27     
+ Misses        206     203       -3
Impacted Files Coverage Δ
src/types.jl 74.77% <100%> (+3.34%) ⬆️
src/breakpoints.jl 92.64% <100%> (+3.67%) ⬆️
src/interpret.jl 86.31% <100%> (ø) ⬆️
src/utils.jl 85.57% <100%> (+0.35%) ⬆️
src/construct.jl 91.9% <97.56%> (-0.5%) ⬇️

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 f9ebc94...0595137. Read the comment docs.

@KristofferC KristofferC changed the title Create a "junk yard" even from Frames Create a "junk yard" even for Frames Jul 29, 2019
@timholy
Copy link
Member

timholy commented Jul 29, 2019

Oh, interesting. Yes, I guess if we speed up the rest, this becomes important.

@KristofferC
Copy link
Member Author

It might improve GC times since we have a lot fewer objects to track as well. But for now, it doesn't seem like that big of a deal.

@timholy
Copy link
Member

timholy commented Jul 30, 2019

I'm guessing the "one long vector" for all slots & ssavalues might supplant the need for this? It would also solve another problem: currently the caller packs the args into a vector, and then when you switch to the new frame they get copied again into the frame's slots. If we had one long vector, we could pack the args into it and thus do it only once.

@KristofferC KristofferC merged commit 02c51a3 into master Aug 16, 2019
@KristofferC KristofferC deleted the kc/frame_junkyard branch August 16, 2019 12:51
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.

3 participants