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

Fix show_unquoted invalidations #47050

Conversation

rikhuijzer
Copy link
Contributor

@rikhuijzer rikhuijzer commented Oct 4, 2022

This PR fixes about 1k method invalidations on Julia nightly for multiple separate packages. The following table lists the number of invalidations on Julia 1.8.2, Julia nightly and this PR:

Code 1.8.2 1.9.0-DEV.1506 This PR
using CSV 3173 5452 4560
using CategoricalArrays 3482 4713 3668
using InlineStrings 1376 654 654
using ProgressLogging 246 2990 2049

Generated with:

using Pkg

Pkg.activate(; temp=true)

Pkg.add(["SnoopCompileCore", "SnoopCompile", "InlineStrings"])

using SnoopCompileCore

invs = @snoopr using InlineStrings

using SnoopCompile

trees = invalidation_trees(invs)

print('\n', trees, '\n')

@show length(invs)

and running for various packages and Julia versions with --startup-file=no.

Note that this PR reverts a change introduced by #44500. It changes a convert(String, X) back to string(X).

Based on this PR reverting an earlier PR and the seemingly random increases and decreases in the table, adding invalidations tests should help avoid regressions (#47022). I'll see whether I can find time to work on that.

cc: @timholy, @ranocha

I suggest the latency label.

This PR fixes about 1000 method invalidations for multiple separate
packages. The following table lists the number of invalidations on
Julia 1.8.2, Julia nightly and this PR:

Name | 1.8.2 | 1.9.0-DEV.1506 | This PR
--- | --- | --- | ---
CSV | 3173 | 5452 | 4560
CategoricalArrays | 3482 | 4713 | 3668
InlineStrings | 1376 | 654 | 654
ProgressLogging | 246 | 2990 | 2049

Generated with:

```julia
using Pkg

Pkg.activate(; temp=true)

Pkg.add(["SnoopCompileCore", "SnoopCompile", "InlineStrings"])

using SnoopCompileCore

invs = @Snoopr using InlineStrings

using SnoopCompile

trees = invalidation_trees(invs)

print('\n', trees, '\n')

@show length(invs)
```

and running the various Julia versions with `--startup-file=no`.

Note that this PR reverts a change introduced by
JuliaLang#44500.
It changes a `convert(String, X)` back to `string(X)`.

Based on this PR reverting an earlier PR and the seemingly random
increases and decreases in the table, adding invalidations tests should
help avoid regressions
(JuliaLang#47022). I'll see whether I
can find time to work on that.
@aviatesk aviatesk added the latency Latency label Oct 5, 2022
@rikhuijzer
Copy link
Contributor Author

Closing in favor of #47093.

@rikhuijzer rikhuijzer closed this Oct 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
latency Latency
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants