Eliminate all Core.Box captures to reduce latency and allocations#5475
Open
hz-xiaxz wants to merge 86 commits intoMakieOrg:masterfrom
Open
Eliminate all Core.Box captures to reduce latency and allocations#5475hz-xiaxz wants to merge 86 commits intoMakieOrg:masterfrom
Core.Box captures to reduce latency and allocations#5475hz-xiaxz wants to merge 86 commits intoMakieOrg:masterfrom
Conversation
USER TTY FROM LOGIN@ IDLE JCPU PCPU WHAT hzxiaxz pts/1 - 13:53 1:29m 0.45s 0.45s -zsh, variables to avoid boxing
Author
|
Motivated by the performance bump, I continued to eliminate all Core.Box found by JETLS. |
Core.Box captures to reduce latency and allocationsCore.Box captures to reduce latency and allocations
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR addresses a significant number of Core.Box captures throughout the codebase. These were identified using the scanning script from JuliaLang/julia#60478.
Core.Boxoccurs when the compiler cannot infer the type or lifetime of a variable captured in a closure (often due to reassignment), forcing it onto the heap. This causes:Type instabilities (inhibiting optimizations like SIMD).
Unnecessary allocations (increasing GC pressure).
Latency (compilation overhead).
While this PR touches many files, the changes are largely mechanical. I hope it won't be a large burden to review.
Strategies Used
To eliminate boxing, I employed several standard techniques depending on the context:
x = ...; x = ...changed tox_init = ...; x_final = ...).Refcontainers: UsingReffor variables that genuinely need to be mutated inside closures (e.g., buffers or counters), keeping the binding constant.mapwith explicitforloops where stateful iteration made closures inefficient.The scan results show a drastic reduction in boxed variables.
Before (Detected Boxes)
pvmf32#553projection_utils.jl:276viewport#Scene#303scenes.jl:236labels#get_labeled_plotslegend.jl:1014lplots#get_labeled_plotslegend.jl:1014pos#initialize_block!textbox.jl:162relayout#initialize_block!legend.jl:42i#initialize_block!menu.jl:1was_inside_button#initialize_block!menu.jl:1was_inside_options#initialize_block!menu.jl:1last_task#on_latestobservables.jl:26run_f#on_latestobservables.jl:26N#plot!contours.jl:180mesh_idx#plot!poly.jl:183xy#position_on_plotray_casting.jl:378nrows#print_columnsrecipes.jl:709new_theme#set_theme!theming.jl:209ticklabelsLineAxislineaxis.jl:264fnStringtext.jl:874result_type_register_argument_conversions!compute-plots.jl:491x_register_argument_conversions!compute-plots.jl:491alignadd_ticks_and_ticklabels!axis3d.jl:592maxiaggregation_implementation!datashader.jl:143miniaggregation_implementation!datashader.jl:143iattribute_per_chartext_layouting.jl:9bar_labelsbarplot_labelsbarplot.jl:262deregisterconnect_conversions!dim-converts.jl:121tfontsizedraw_axis3daxis.jl:226caminitialize_block!axis3d.jl:3default_formatinitialize_block!slidergrid.jl:8extract_range_formatinitialize_block!slidergrid.jl:8reset_to_storedinitialize_block!textbox.jl:4textplotinitialize_block!textbox.jl:4x1line_rectangle_intersectionannotation.jl:920x2line_rectangle_intersectionannotation.jl:920y1line_rectangle_intersectionannotation.jl:920y2line_rectangle_intersectionannotation.jl:920deregisterneeds_tick_update_observabledim-converts.jl:160lastcommparse_bezier_commandsbezier.jl:401bufferplot!timeseries.jl:39nanpointplot!band.jl:36hprojectionmatrixaxis3d.jl:331wprojectionmatrixaxis3d.jl:331fieldreplay_eventsevent-recorder.jl:32valuereplay_eventsevent-recorder.jl:32indstreamplot_implstreamplot.jl:134c1to_imagepatterns.jl:126c2to_imagepatterns.jl:126bbunchecked_boundingboxtext_boundingbox.jl:28update_plotlistupdate_plotspecs!specapi.jl:666colorvoxel_colorsvoxels.jl:282After: No
We can add a
Test.detect_closure_boxesin tests to avoid introducing more boxes.Type of change
Delete options that do not apply:
AI Disclose
I used some AI to assist me, but checked manually.
Checklist