Skip to content

Also hash spans inside the same file as relative (V2) #150540

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.

Sign up for GitHub

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
rust-bors merged 1 commit into from
Jan 11, 2026
+58 −39
Merged

Also hash spans inside the same file as relative (V2) #150540

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 26 additions & 20 deletions compiler/rustc_middle/src/query/on_disk_cache.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ use rustc_span::hygiene::{
};
use rustc_span::source_map::Spanned;
use rustc_span::{
BlobDecoder, BytePos, ByteSymbol, CachingSourceMapView, ExpnData, ExpnHash, Pos,
RelativeBytePos, SourceFile, Span, SpanDecoder, SpanEncoder, StableSourceFileId, Symbol,
BlobDecoder, BytePos, ByteSymbol, CachingSourceMapView, ExpnData, ExpnHash, RelativeBytePos,
SourceFile, Span, SpanDecoder, SpanEncoder, StableSourceFileId, Symbol,
};

use crate::dep_graph::{DepNodeIndex, SerializedDepNodeIndex};
Expand Down Expand Up @@ -652,7 +652,10 @@ impl<'a, 'tcx> SpanDecoder for CacheDecoder<'a, 'tcx> {
let dto = u32::decode(self);

let enclosing = self.tcx.source_span_untracked(parent.unwrap()).data_untracked();
(enclosing.lo + BytePos::from_u32(dlo), enclosing.lo + BytePos::from_u32(dto))
(
BytePos(enclosing.lo.0.wrapping_add(dlo)),
BytePos(enclosing.lo.0.wrapping_add(dto)),
)
}
TAG_FULL_SPAN => {
let file_lo_index = SourceFileIndex::decode(self);
Expand Down Expand Up @@ -894,30 +897,33 @@ impl<'a, 'tcx> SpanEncoder for CacheEncoder<'a, 'tcx> {
return TAG_PARTIAL_SPAN.encode(self);
}

if let Some(parent) = span_data.parent {
let enclosing = self.tcx.source_span_untracked(parent).data_untracked();
if enclosing.contains(span_data) {
TAG_RELATIVE_SPAN.encode(self);
(span_data.lo - enclosing.lo).to_u32().encode(self);
(span_data.hi - enclosing.lo).to_u32().encode(self);
return;
}
let parent =
span_data.parent.map(|parent| self.tcx.source_span_untracked(parent).data_untracked());
if let Some(parent) = parent
&& parent.contains(span_data)
{
TAG_RELATIVE_SPAN.encode(self);
(span_data.lo.0.wrapping_sub(parent.lo.0)).encode(self);
(span_data.hi.0.wrapping_sub(parent.lo.0)).encode(self);
return;
}

let pos = self.source_map.byte_pos_to_line_and_col(span_data.lo);
let partial_span = match &pos {
Some((file_lo, _, _)) => !file_lo.contains(span_data.hi),
None => true,
let Some((file_lo, line_lo, col_lo)) =
self.source_map.byte_pos_to_line_and_col(span_data.lo)
else {
return TAG_PARTIAL_SPAN.encode(self);
};

if partial_span {
return TAG_PARTIAL_SPAN.encode(self);
if let Some(parent) = parent
&& file_lo.contains(parent.lo)
{
TAG_RELATIVE_SPAN.encode(self);
Copy link
Contributor Author

@JonathanBrouwer JonathanBrouwer Jan 8, 2026

Choose a reason for hiding this comment

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

Your PR introduces a new TAG_RELATIVE_OUTER_SPAN tag here for encoding spans which are outside their parent span, this PR instead reuses the TAG_RELATIVE_SPAN tag. This can be done by encoding the positions in the span (which are u32s) subtracted from their parent, wrapping if necessary. This change seems to be most of the performance gained relative to your PR.

(span_data.lo.0.wrapping_sub(parent.lo.0)).encode(self);
(span_data.hi.0.wrapping_sub(parent.lo.0)).encode(self);
return;
}

let (file_lo, line_lo, col_lo) = pos.unwrap();

let len = span_data.hi - span_data.lo;

let source_file_index = self.source_file_index(file_lo);

TAG_FULL_SPAN.encode(self);
Expand Down
6 changes: 2 additions & 4 deletions compiler/rustc_query_system/src/ich/hcx.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,7 @@ use rustc_hir::definitions::DefPathHash;
use rustc_session::Session;
use rustc_session::cstore::Untracked;
use rustc_span::source_map::SourceMap;
use rustc_span::{
BytePos, CachingSourceMapView, DUMMY_SP, Span, SpanData, StableSourceFileId, Symbol,
};
use rustc_span::{BytePos, CachingSourceMapView, DUMMY_SP, SourceFile, Span, SpanData, Symbol};

use crate::ich;

Expand Down Expand Up @@ -118,7 +116,7 @@ impl<'a> rustc_span::HashStableContext for StableHashingContext<'a> {
fn span_data_to_lines_and_cols(
&mut self,
span: &SpanData,
) -> Option<(StableSourceFileId, usize, BytePos, usize, BytePos)> {
) -> Option<(&SourceFile, usize, BytePos, usize, BytePos)> {
self.source_map().span_data_to_lines_and_cols(span)
}

Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_span/src/caching_source_map_view.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::ops::Range;
use std::sync::Arc;

use crate::source_map::SourceMap;
use crate::{BytePos, Pos, RelativeBytePos, SourceFile, SpanData, StableSourceFileId};
use crate::{BytePos, Pos, RelativeBytePos, SourceFile, SpanData};

#[derive(Clone)]
struct CacheEntry {
Expand Down Expand Up @@ -114,7 +114,7 @@ impl<'sm> CachingSourceMapView<'sm> {
pub fn span_data_to_lines_and_cols(
&mut self,
span_data: &SpanData,
) -> Option<(StableSourceFileId, usize, BytePos, usize, BytePos)> {
) -> Option<(&SourceFile, usize, BytePos, usize, BytePos)> {
self.time_stamp += 1;

// Check if lo and hi are in the cached lines.
Expand All @@ -136,7 +136,7 @@ impl<'sm> CachingSourceMapView<'sm> {
let lo = &self.line_cache[lo_cache_idx as usize];
let hi = &self.line_cache[hi_cache_idx as usize];
return Some((
lo.file.stable_id,
&lo.file,
lo.line_number,
span_data.lo - lo.line.start,
hi.line_number,
Expand Down Expand Up @@ -224,7 +224,7 @@ impl<'sm> CachingSourceMapView<'sm> {
assert_eq!(lo.file_index, hi.file_index);

Some((
lo.file.stable_id,
&lo.file,
lo.line_number,
span_data.lo - lo.line.start,
hi.line_number,
Expand Down
37 changes: 26 additions & 11 deletions compiler/rustc_span/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2831,7 +2831,7 @@ pub trait HashStableContext {
fn span_data_to_lines_and_cols(
&mut self,
span: &SpanData,
) -> Option<(StableSourceFileId, usize, BytePos, usize, BytePos)>;
) -> Option<(&SourceFile, usize, BytePos, usize, BytePos)>;
fn hashing_controls(&self) -> HashingControls;
}

Expand All @@ -2849,6 +2849,8 @@ where
/// codepoint offsets. For the purpose of the hash that's sufficient.
/// Also, hashing filenames is expensive so we avoid doing it twice when the
/// span starts and ends in the same file, which is almost always the case.
///
/// IMPORTANT: changes to this method should be reflected in implementations of `SpanEncoder`.
fn hash_stable(&self, ctx: &mut CTX, hasher: &mut StableHasher) {
const TAG_VALID_SPAN: u8 = 0;
const TAG_INVALID_SPAN: u8 = 1;
Expand All @@ -2867,15 +2869,17 @@ where
return;
}

if let Some(parent) = span.parent {
let def_span = ctx.def_span(parent).data_untracked();
if def_span.contains(span) {
// This span is enclosed in a definition: only hash the relative position.
Hash::hash(&TAG_RELATIVE_SPAN, hasher);
(span.lo - def_span.lo).to_u32().hash_stable(ctx, hasher);
(span.hi - def_span.lo).to_u32().hash_stable(ctx, hasher);
return;
}
let parent = span.parent.map(|parent| ctx.def_span(parent).data_untracked());
if let Some(parent) = parent
&& parent.contains(span)
{
// This span is enclosed in a definition: only hash the relative position.
// This catches a subset of the cases from the `file.contains(parent.lo)`,
// But we can do this check cheaply without the expensive `span_data_to_lines_and_cols` query.
Hash::hash(&TAG_RELATIVE_SPAN, hasher);
(span.lo - parent.lo).to_u32().hash_stable(ctx, hasher);
(span.hi - parent.lo).to_u32().hash_stable(ctx, hasher);
return;
}

// If this is not an empty or invalid span, we want to hash the last
Expand All @@ -2887,8 +2891,19 @@ where
return;
};

if let Some(parent) = parent
&& file.contains(parent.lo)
{
// This span is relative to another span in the same file,
// only hash the relative position.
Hash::hash(&TAG_RELATIVE_SPAN, hasher);
Hash::hash(&(span.lo.0.wrapping_sub(parent.lo.0)), hasher);
Copy link
Contributor Author

@JonathanBrouwer JonathanBrouwer Jan 8, 2026

Choose a reason for hiding this comment

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

When hashing, your PR hashes an isize here, this PR instead hashes a u32 by using a wrapping_sub. Haven't explored if this makes any perf difference

Hash::hash(&(span.hi.0.wrapping_sub(parent.lo.0)), hasher);
return;
}

Hash::hash(&TAG_VALID_SPAN, hasher);
Hash::hash(&file, hasher);
Hash::hash(&file.stable_id, hasher);
Copy link
Contributor Author

@JonathanBrouwer JonathanBrouwer Jan 8, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Your PR hashed the TAG_VALID_SPAN and stable_id if file.contains(parent.lo), this PR doesn't. Again, Haven't explored if this makes any perf difference


// Hash both the length and the end location (line/column) of a span. If we
// hash only the length, for example, then two otherwise equal spans with
Expand Down
Loading