Skip to content

Conversation

nsajko
Copy link
Member

@nsajko nsajko commented Mar 2, 2025

The fields of StringIndexError are abstractly typed, so there's no reason to specialize on a concrete type, I think. The change seems like it could prevent some invalidation on loading user code.

@nsajko nsajko added error handling Handling of exceptions by Julia or the user strings "Strings!" invalidations labels Mar 2, 2025
@nsajko nsajko force-pushed the Base_strings_StringIndexError_nospecialize branch from 7e082a5 to ae2ee2a Compare March 2, 2025 11:33
@nsajko nsajko added the DO NOT MERGE Do not merge this PR! label Mar 2, 2025
@LilithHafner LilithHafner marked this pull request as draft March 2, 2025 12:49
@nsajko nsajko removed the DO NOT MERGE Do not merge this PR! label Mar 2, 2025
@nsajko nsajko marked this pull request as ready for review March 2, 2025 13:53
@nsajko
Copy link
Member Author

nsajko commented Mar 2, 2025

The change decreases the invalidation count on loading package TypeDomainNaturalNumbers v6.1.0 (without first loading the REPL) from 989 to 980 needs updating.

Reproducer:

./julia -E 'using SnoopCompileCore; i=(@snoop_invalidations using TypeDomainNaturalNumbers); using SnoopCompile; length(uinvalidated(i))'

@nsajko nsajko force-pushed the Base_strings_StringIndexError_nospecialize branch from 8044dba to dc76a47 Compare March 3, 2025 12:17
nsajko added 3 commits March 8, 2025 20:53
The fields of `StringIndexError` are abstractly typed, so there's no
reason to specialize, I think. The change seems like it could prevent
some invalidation on loading user code.
@nsajko nsajko force-pushed the Base_strings_StringIndexError_nospecialize branch from 4a45773 to 7f373e3 Compare March 8, 2025 19:54
@nsajko nsajko changed the title @nospecialize for string_index_err and StringIndexError @nospecialize for string_index_err Mar 8, 2025
@nsajko nsajko added backport 1.10 Change should be backported to the 1.10 release backport 1.11 Change should be backported to release-1.11 backport 1.12 Change should be backported to release-1.12 labels Mar 9, 2025
This was referenced Mar 11, 2025
This was referenced Mar 20, 2025
@KristofferC KristofferC mentioned this pull request Mar 31, 2025
36 tasks
@@ -9,7 +9,7 @@ struct StringIndexError <: Exception
string::AbstractString
index::Integer
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at string_index_err this could be made ::Int I think?

@KristofferC KristofferC mentioned this pull request Apr 4, 2025
51 tasks
@KristofferC KristofferC mentioned this pull request Apr 25, 2025
71 tasks
@KristofferC KristofferC mentioned this pull request Apr 29, 2025
53 tasks
@KristofferC KristofferC mentioned this pull request May 9, 2025
58 tasks
@KristofferC KristofferC mentioned this pull request Jun 6, 2025
60 tasks
@KristofferC KristofferC mentioned this pull request Jul 22, 2025
20 tasks
@KristofferC KristofferC mentioned this pull request Aug 6, 2025
38 tasks
This was referenced Aug 19, 2025
@aviatesk aviatesk merged commit bc33a3e into JuliaLang:master Aug 21, 2025
7 checks passed
aviatesk added a commit that referenced this pull request Aug 21, 2025
The fields of `StringIndexError` are abstractly typed, so there's no
reason to specialize on a concrete type. The change seems like
it could prevent some invalidation on loading user code.

---------

Co-authored-by: Shuhei Kadowaki <[email protected]>
@nsajko nsajko deleted the Base_strings_StringIndexError_nospecialize branch August 21, 2025 11:43
KristofferC pushed a commit that referenced this pull request Sep 2, 2025
The fields of `StringIndexError` are abstractly typed, so there's no
reason to specialize on a concrete type. The change seems like
it could prevent some invalidation on loading user code.

---------

Co-authored-by: Shuhei Kadowaki <[email protected]>
DilumAluthge added a commit that referenced this pull request Sep 5, 2025
Backported PRs:
- [x] #54840 <!-- Add boundscheck in speccache_eq to avoid OOB access
due to data race -->
- [x] #42080 <!-- recommend explicit `using Foo: Foo, ...` in package
code (was: "using considered harmful") -->
- [x] #58127 <!-- [DOC] Update installation docs: /downloads/ =>
/install/ -->
- [x] #58202 <!-- [release-1.11] malloc: use jl_get_current_task to fix
null check -->
- [x] #58584 <!-- Make `Ptr` values static-show w/ type-information -->
- [x] #58637 <!-- Make late gc lower handle insertelement of alloca use.
-->
- [x] #58837 <!-- fix null comparisons for non-standard address spaces
-->
- [x] #57826 <!-- Add a `similar` method for `Type{<:CodeUnits}` -->
- [x] #58293 <!-- fix trailing indices stackoverflow in reinterpreted
array -->
- [x] #58887 <!-- Pkg: Allow configuring can_fancyprint(io::IO) using
IOContext -->
- [x] #58937 <!-- Fix nthreadpools size in JLOptions -->
- [x] #58978 <!-- Fix precompilepkgs warn loaded setting -->
- [x] #58998 <!-- Bugfix: Use Base.aligned_sizeof instead of sizeof in
Mmap.mmap -->
- [x] #59120 <!-- Fix memory order typo in "src/julia_atomics.h" -->
- [x] #59170 <!-- Clarify and enhance confusing precompile test -->

Need manual backport:
- [ ] #56329 <!-- loading: clean up more concurrency issues -->
- [ ] #56956 <!-- Add "mea culpa" to foreign module assignment error.
-->
- [ ] #57035 <!-- linux: workaround to avoid deadlock inside
dl_iterate_phdr in glibc -->
- [ ] #57089 <!-- Block thread from receiving profile signal with
stackwalk lock -->
- [ ] #57249 <!-- restore non-freebsd-unix fix for profiling -->
- [ ] #58011 <!-- Remove try-finally scope from `@time_imports`
`@trace_compile` `@trace_dispatch` -->
- [ ] #58062 <!-- remove unnecessary edge from `exp_impl` to `pow` -->
- [ ] #58157 <!-- add showing a string to REPL precompile workload -->
- [ ] #58209 <!-- Specialize `one` for the `SizedArray` test helper -->
- [ ] #58108 <!-- Base.get_extension & Dates.format made public -->
- [ ] #58356 <!-- codegen: remove readonly from abstract type calling
convention -->
- [ ] #58415 <!-- [REPL] more reliable extension loading -->
- [ ] #58510 <!-- Don't filter `Core` methods from newly-inferred list
-->
- [ ] #58110 <!-- relax dispatch for the `IteratorSize` method for
`Generator` -->
- [ ] #58965 <!-- Fix `hygienic-scope`s in inner macro expansions -->
- [ ] #58971 <!-- Fix alignment of failed precompile jobs on CI -->
- [ ] #59066 <!-- build: Also pass -fno-strict-aliasing for C++ -->

Contains multiple commits, manual intervention needed:
- [ ] #55877 <!-- fix FileWatching designs and add workaround for a stat
bug on Apple -->
- [ ] #56755 <!-- docs: fix scope type of a `struct` to hard -->
- [ ] #57809 <!-- Fix fptrunc Float64 -> Float16 rounding through
Float32 -->
- [ ] #57398 <!-- Make remaining float intrinsics require float
arguments -->
- [ ] #56351 <!-- Fix `--project=@script` when outside script directory
-->
- [ ] #57129 <!-- clarify that time_ns is monotonic -->
- [ ] #58134 <!-- Note annotated string API is experimental in Julia
1.11 in HISTORY.md -->
- [ ] #58401 <!-- check that hashing of types does not foreigncall
(`jl_type_hash` is concrete evaluated) -->
- [ ] #58435 <!-- Fix layout flags for types that have oddly sized
primitive type fields -->
- [ ] #58483 <!-- Fix tbaa usage when storing into heap allocated
immutable structs -->
- [ ] #58512 <!-- Make more types jl_static_show readably -->
- [ ] #58012 <!-- Re-enable tab completion of kwargs for large method
tables -->
- [ ] #58683 <!-- Add 0 predecessor to entry basic block and handle it
in inlining -->
- [ ] #59112 <!-- Add builtin function name to add methods error -->

Non-merged PRs with backport label:
- [ ] #59329 <!-- aotcompile: destroy LLVM context after serializing
combined module -->
- [ ] #58848 <!-- Set array size only when safe to do so -->
- [ ] #58535 <!-- gf.c: include const-return methods in
`--trace-compile` -->
- [ ] #58038 <!-- strings/cstring: `transcode`: prevent Windows sysimage
invalidation -->
- [ ] #57604 <!-- `@nospecialize` for `string_index_err` -->
- [ ] #57366 <!-- Use ptrdiff_t sized offsets for gvars_offsets to allow
large sysimages -->
- [ ] #56890 <!-- Enable getting non-boxed LLVM type from Julia Type -->
- [ ] #56823 <!-- Make version of opaque closure constructor in world
-->
- [ ] #55958 <!-- also redirect JL_STDERR etc. when redirecting to
devnull -->
- [ ] #55956 <!-- Make threadcall gc safe -->
- [ ] #55534 <!-- Set stdlib sources as read-only during installation
-->
- [ ] #55499 <!-- propagate the terminal's `displaysize` to the
`IOContext` used by the REPL -->
- [ ] #55458 <!-- Allow for generically extracting unannotated string
-->
- [ ] #55457 <!-- Make AnnotateChar equality consider annotations -->
- [ ] #55220 <!-- `isfile_casesensitive` fixes on Windows -->
- [ ] #53957 <!-- tweak how filtering is done for what packages should
be precompiled -->
- [ ] #51479 <!-- prevent code loading from lookin in the versioned
environment when building Julia -->
- [ ] #50813 <!-- More doctests for Sockets and capitalization fix -->
- [ ] #50157 <!-- improve docs for `@inbounds` and
`Base.@propagate_inbounds` -->

---------

Co-authored-by: Kiran Pamnany <[email protected]>
Co-authored-by: adienes <[email protected]>
Co-authored-by: Gabriel Baraldi <[email protected]>
Co-authored-by: Keno Fischer <[email protected]>
Co-authored-by: Simeon David Schaub <[email protected]>
Co-authored-by: Jameson Nash <[email protected]>
Co-authored-by: Alex Arslan <[email protected]>
Co-authored-by: Fons van der Plas <[email protected]>
Co-authored-by: Ian Butterworth <[email protected]>
Co-authored-by: JonasIsensee <[email protected]>
Co-authored-by: Curtis Vogt <[email protected]>
Co-authored-by: Dilum Aluthge <[email protected]>
Co-authored-by: DilumAluthgeBot <[email protected]>
Co-authored-by: DilumAluthge <[email protected]>
DilumAluthge pushed a commit that referenced this pull request Sep 9, 2025
The fields of `StringIndexError` are abstractly typed, so there's no
reason to specialize on a concrete type. The change seems like
it could prevent some invalidation on loading user code.

---------

Co-authored-by: Shuhei Kadowaki <[email protected]>
(cherry picked from commit bc33a3e)
@DilumAluthge DilumAluthge mentioned this pull request Sep 9, 2025
65 tasks
@DilumAluthge DilumAluthge removed the backport 1.11 Change should be backported to release-1.11 label Sep 9, 2025
@KristofferC KristofferC removed the backport 1.12 Change should be backported to release-1.12 label Sep 11, 2025
@DilumAluthge DilumAluthge added the backport 1.11 Change should be backported to release-1.11 label Sep 11, 2025
DilumAluthge pushed a commit that referenced this pull request Sep 13, 2025
The fields of `StringIndexError` are abstractly typed, so there's no
reason to specialize on a concrete type. The change seems like
it could prevent some invalidation on loading user code.

---------

Co-authored-by: Shuhei Kadowaki <[email protected]>
(cherry picked from commit bc33a3e)
KristofferC pushed a commit that referenced this pull request Sep 15, 2025
The fields of `StringIndexError` are abstractly typed, so there's no
reason to specialize on a concrete type. The change seems like
it could prevent some invalidation on loading user code.

---------

Co-authored-by: Shuhei Kadowaki <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 1.10 Change should be backported to the 1.10 release backport 1.11 Change should be backported to release-1.11 error handling Handling of exceptions by Julia or the user invalidations strings "Strings!"
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants