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.

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

Jump to bottom

[rustdoc] Use Map instead of Object for source files and search index #118910

Merged
merged 1 commit into from Dec 14, 2023
Merged

[rustdoc] Use Map instead of Object for source files and search index #118910

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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/librustdoc/html/render/search_index.rs
View file
Open in desktop
Expand Up @@ -488,7 +488,7 @@ pub(crate) fn build_index<'tcx>(

// Collect the index into a string
format!(
r#""{}":{}"#,
r#"["{}",{}]"#,
Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer not to make changes that add more bytes to these files. Instead, could we keep the object representation in JSON and add a function to convert to a map at the end?

function rustdocObjectToMap(obj) {
    return new Map(Object.entries(obj));
}

Copy link
Member Author

Choose a reason for hiding this comment

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

Hum, that kinda kills the original point. Do you mind running a perf check on this already before I make the change please? Like that we'll be able to see if how we transform into a Map changes anything.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, that makes sense.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh I meant on your JS perf check (I don't think there will be any change on the rust side). I couldn't find it on your profile. ^^'

Copy link
Contributor

Choose a reason for hiding this comment

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

https://gitlab.com/notriddle/rustdoc-js-profile

Copy link
Member Author

Choose a reason for hiding this comment

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

I was looking on github so now I understand why I didn't find it... Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

There doesn’t seem to be any significant difference in doc bytes.

I’ll try running the performance test on it as soon as the merge conflicts are resolved.

krate.name(tcx),
serde_json::to_string(&CrateData {
doc: crate_doc,
Expand Down
19 changes: 10 additions & 9 deletions src/librustdoc/html/render/write_shared.rs
View file
Open in desktop
Expand Up @@ -167,23 +167,24 @@ pub(super) fn write_shared(
let mut krates = Vec::new();

if path.exists() {
let prefix = format!("\"{krate}\"");
let prefix = format!("[\"{krate}\"");
for line in BufReader::new(File::open(path)?).lines() {
let line = line?;
if !line.starts_with('"') {
if !line.starts_with("[\"") {
continue;
}
if line.starts_with(&prefix) {
continue;
}
if line.ends_with(",\\") {
if line.ends_with("],\\") {
ret.push(line[..line.len() - 2].to_string());
} else {
// Ends with "\\" (it's the case for the last added crate line)
ret.push(line[..line.len() - 1].to_string());
}
krates.push(
line.split('"')
line[1..] // We skip the `[` parent at the beginning of the line.
.split('"')
.find(|s| !s.is_empty())
.map(|s| s.to_owned())
.unwrap_or_else(String::new),
Expand Down Expand Up @@ -285,7 +286,7 @@ pub(super) fn write_shared(
let (mut all_sources, _krates) =
try_err!(collect_json(&dst, krate.name(cx.tcx()).as_str()), &dst);
all_sources.push(format!(
r#""{}":{}"#,
r#"["{}",{}]"#,
&krate.name(cx.tcx()),
hierarchy
.to_json_string()
Expand All @@ -296,9 +297,9 @@ pub(super) fn write_shared(
.replace("\\\"", "\\\\\"")
));
all_sources.sort();
let mut v = String::from("var srcIndex = JSON.parse('{\\\n");
let mut v = String::from("const srcIndex = new Map(JSON.parse('[\\\n");
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, crud.

The test cases pass, but this has a race condition. I'll open a PR to fix it in a few minutes.

v.push_str(&all_sources.join(",\\\n"));
v.push_str("\\\n}');\ncreateSrcSidebar();\n");
v.push_str("\\\n]'));\ncreateSrcSidebar();\n");
Ok(v.into_bytes())
};
write_invocation_specific("src-files.js", &make_sources)?;
Expand All @@ -316,11 +317,11 @@ pub(super) fn write_shared(
// with rustdoc running in parallel.
all_indexes.sort();
write_invocation_specific("search-index.js", &|| {
let mut v = String::from("var searchIndex = JSON.parse('{\\\n");
let mut v = String::from("const searchIndex = new Map(JSON.parse('[\\\n");
v.push_str(&all_indexes.join(",\\\n"));
v.push_str(
r#"\
}');
]'));
if (typeof window !== 'undefined' && window.initSearch) {window.initSearch(searchIndex)};
if (typeof exports !== 'undefined') {exports.searchIndex = searchIndex};
"#,
Expand Down
141 changes: 62 additions & 79 deletions src/librustdoc/html/static/js/search.js
View file
Open in desktop
Expand Up @@ -80,10 +80,6 @@ const longItemTypes = [
const TY_GENERIC = itemTypes.indexOf("generic");
const ROOT_PATH = typeof window !== "undefined" ? window.rootPath : "../";

function hasOwnPropertyRustdoc(obj, property) {
return Object.prototype.hasOwnProperty.call(obj, property);
}

// In the search display, allows to switch between tabs.
function printTab(nb) {
let iter = 0;
Expand Down Expand Up @@ -1074,7 +1070,7 @@ function initSearch(rawSearchIndex) {

if (elem &&
elem.value !== "all crates" &&
hasOwnPropertyRustdoc(rawSearchIndex, elem.value)
rawSearchIndex.has(elem.value)
) {
return elem.value;
}
Expand Down Expand Up @@ -2524,11 +2520,10 @@ ${item.displayPath}<span class="${type}">${name}</span>\
}

let crates = "";
const crates_list = Object.keys(rawSearchIndex);
if (crates_list.length > 1) {
if (rawSearchIndex.size > 1) {
crates = " in&nbsp;<div id=\"crate-search-div\"><select id=\"crate-search\">" +
"<option value=\"all crates\">all crates</option>";
for (const c of crates_list) {
for (const c of rawSearchIndex.keys()) {
crates += `<option value="${c}" ${c === filterCrates && "selected"}>${c}</option>`;
}
crates += "</select></div>";
Expand Down Expand Up @@ -2945,81 +2940,70 @@ ${item.displayPath}<span class="${type}">${name}</span>\
// Function type fingerprints are 128-bit bloom filters that are used to
// estimate the distance between function and query.
// This loop counts the number of items to allocate a fingerprint for.
for (const crate in rawSearchIndex) {
if (!hasOwnPropertyRustdoc(rawSearchIndex, crate)) {
continue;
}
for (const crate of rawSearchIndex.values()) {
// Each item gets an entry in the fingerprint array, and the crate
// does, too
id += rawSearchIndex[crate].t.length + 1;
id += crate.t.length + 1;
}
functionTypeFingerprint = new Uint32Array((id + 1) * 4);

// This loop actually generates the search item indexes, including
// normalized names, type signature objects and fingerprints, and aliases.
id = 0;
for (const crate in rawSearchIndex) {
if (!hasOwnPropertyRustdoc(rawSearchIndex, crate)) {
continue;
}

let crateSize = 0;

/**
* The raw search data for a given crate. `n`, `t`, `d`, `i`, and `f`
* are arrays with the same length. `q`, `a`, and `c` use a sparse
* representation for compactness.
*
* `n[i]` contains the name of an item.
*
* `t[i]` contains the type of that item
* (as a string of characters that represent an offset in `itemTypes`).
*
* `d[i]` contains the description of that item.
*
* `q` contains the full paths of the items. For compactness, it is a set of
* (index, path) pairs used to create a map. If a given index `i` is
* not present, this indicates "same as the last index present".
*
* `i[i]` contains an item's parent, usually a module. For compactness,
* it is a set of indexes into the `p` array.
*
* `f[i]` contains function signatures, or `0` if the item isn't a function.
* Functions are themselves encoded as arrays. The first item is a list of
* types representing the function's inputs, and the second list item is a list
* of types representing the function's output. Tuples are flattened.
* Types are also represented as arrays; the first item is an index into the `p`
* array, while the second is a list of types representing any generic parameters.
*
* b[i] contains an item's impl disambiguator. This is only present if an item
* is defined in an impl block and, the impl block's type has more than one associated
* item with the same name.
*
* `a` defines aliases with an Array of pairs: [name, offset], where `offset`
* points into the n/t/d/q/i/f arrays.
*
* `doc` contains the description of the crate.
*
* `p` is a list of path/type pairs. It is used for parents and function parameters.
*
* `c` is an array of item indices that are deprecated.
*
* @type {{
* doc: string,
* a: Object,
* n: Array<string>,
* t: String,
* d: Array<string>,
* q: Array<[Number, string]>,
* i: Array<Number>,
* f: Array<RawFunctionSearchType>,
* p: Array<Object>,
* b: Array<[Number, String]>,
* c: Array<Number>
* }}
*/
const crateCorpus = rawSearchIndex[crate];

/**
* The raw search data for a given crate. `n`, `t`, `d`, `i`, and `f`
* are arrays with the same length. `q`, `a`, and `c` use a sparse
* representation for compactness.
*
* `n[i]` contains the name of an item.
*
* `t[i]` contains the type of that item
* (as a string of characters that represent an offset in `itemTypes`).
*
* `d[i]` contains the description of that item.
*
* `q` contains the full paths of the items. For compactness, it is a set of
* (index, path) pairs used to create a map. If a given index `i` is
* not present, this indicates "same as the last index present".
*
* `i[i]` contains an item's parent, usually a module. For compactness,
* it is a set of indexes into the `p` array.
*
* `f[i]` contains function signatures, or `0` if the item isn't a function.
* Functions are themselves encoded as arrays. The first item is a list of
* types representing the function's inputs, and the second list item is a list
* of types representing the function's output. Tuples are flattened.
* Types are also represented as arrays; the first item is an index into the `p`
* array, while the second is a list of types representing any generic parameters.
*
* b[i] contains an item's impl disambiguator. This is only present if an item
* is defined in an impl block and, the impl block's type has more than one associated
* item with the same name.
*
* `a` defines aliases with an Array of pairs: [name, offset], where `offset`
* points into the n/t/d/q/i/f arrays.
*
* `doc` contains the description of the crate.
*
* `p` is a list of path/type pairs. It is used for parents and function parameters.
*
* `c` is an array of item indices that are deprecated.
*
* @type {{
* doc: string,
* a: Object,
* n: Array<string>,
* t: String,
* d: Array<string>,
* q: Array<[Number, string]>,
* i: Array<Number>,
* f: Array<RawFunctionSearchType>,
* p: Array<Object>,
* b: Array<[Number, String]>,
* c: Array<Number>
* }}
*/
for (const [crate, crateCorpus] of rawSearchIndex) {
searchWords.push(crate);
// This object should have exactly the same set of fields as the "row"
// object defined below. Your JavaScript runtime will thank you.
Expand Down Expand Up @@ -3145,14 +3129,13 @@ ${item.displayPath}<span class="${type}">${name}</span>\
id += 1;
searchIndex.push(row);
lastPath = row.path;
crateSize += 1;
}

if (aliases) {
const currentCrateAliases = new Map();
ALIASES.set(crate, currentCrateAliases);
for (const alias_name in aliases) {
if (!hasOwnPropertyRustdoc(aliases, alias_name)) {
if (!Object.prototype.hasOwnProperty.call(aliases, alias_name)) {
continue;
}

Expand All @@ -3168,7 +3151,7 @@ ${item.displayPath}<span class="${type}">${name}</span>\
}
}
}
currentIndex += crateSize;
currentIndex += itemTypes.length;
}
return searchWords;
}
Expand Down Expand Up @@ -3377,7 +3360,7 @@ if (typeof window !== "undefined") {
} else {
// Running in Node, not a browser. Run initSearch just to produce the
// exports.
initSearch({});
initSearch(new Map());
}


Expand Down
8 changes: 4 additions & 4 deletions src/librustdoc/html/static/js/src-script.js
View file
Open in desktop
Expand Up @@ -118,10 +118,10 @@ function createSrcSidebar() {
title.className = "title";
title.innerText = "Files";
sidebar.appendChild(title);
Object.keys(srcIndex).forEach(key => {
srcIndex[key][NAME_OFFSET] = key;
hasFoundFile = createDirEntry(srcIndex[key], sidebar, "", hasFoundFile);
});
for (const [key, source] of srcIndex) {
source[NAME_OFFSET] = key;
hasFoundFile = createDirEntry(source, sidebar, "", hasFoundFile);
}

container.appendChild(sidebar);
// Focus on the current file in the source files sidebar.
Expand Down