Skip to content

Commit 02d5cb0

Browse files
committed
Expose file size from TextStorage
Since it's for telemetry, it prefers to return a stale value rather than triggering file IO (though it will load the file if not even a stale value is available).
1 parent fe26370 commit 02d5cb0

File tree

2 files changed

+133
-16
lines changed

2 files changed

+133
-16
lines changed

src/server/scriptInfo.ts

Lines changed: 47 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,14 @@ namespace ts.server {
2525
*/
2626
private lineMap: number[] | undefined;
2727

28+
/**
29+
* When a large file is loaded, text will artificially be set to "".
30+
* In order to be able to report correct telemetry, we store the actual
31+
* file size in this case. (In other cases where text === "", e.g.
32+
* for mixed content or dynamic files, fileSize will be undefined.)
33+
*/
34+
private fileSize: number | undefined;
35+
2836
/**
2937
* True if the text is for the file thats open in the editor
3038
*/
@@ -56,10 +64,11 @@ namespace ts.server {
5664
this.switchToScriptVersionCache();
5765
}
5866

59-
public useText(newText?: string) {
67+
public useText_TestOnly(newText?: string) {
6068
this.svc = undefined;
6169
this.text = newText;
6270
this.lineMap = undefined;
71+
this.fileSize = undefined;
6372
this.version.text++;
6473
}
6574

@@ -68,37 +77,51 @@ namespace ts.server {
6877
this.ownFileText = false;
6978
this.text = undefined;
7079
this.lineMap = undefined;
80+
this.fileSize = undefined;
7181
}
7282

7383
/**
7484
* Set the contents as newText
7585
* returns true if text changed
7686
*/
77-
public reload(newText: string) {
87+
public reload(newText: string): boolean {
7888
Debug.assert(newText !== undefined);
7989

8090
// Reload always has fresh content
8191
this.pendingReloadFromDisk = false;
8292

93+
// We only need both this.text and this.fileSize if this.text
94+
// is artificially empty because it was too large.
95+
// We assume that an empty string passed to reload is not
96+
// for a file that was too large to store.
97+
this.fileSize = undefined;
98+
8399
// If text changed set the text
84100
// This also ensures that if we had switched to version cache,
85101
// we are switching back to text.
86102
// The change to version cache will happen when needed
87103
// Thus avoiding the computation if there are no changes
88104
if (this.text !== newText) {
89-
this.useText(newText);
105+
this.text = newText;
106+
this.version.text++;
107+
this.svc = undefined;
108+
this.lineMap = undefined;
90109
// We cant guarantee new text is own file text
91110
this.ownFileText = false;
92111
return true;
93112
}
113+
114+
return false;
94115
}
95116

96117
/**
97118
* Reads the contents from tempFile(if supplied) or own file and sets it as contents
98119
* returns true if text changed
99120
*/
100121
public reloadWithFileText(tempFileName?: string) {
101-
const reloaded = this.reload(this.getFileText(tempFileName));
122+
const { text: newText, fileSize } = this.getFileTextAndSize(tempFileName);
123+
const reloaded = this.reload(newText);
124+
this.fileSize = fileSize; // NB: after reload since reload clears it
102125
this.ownFileText = !tempFileName || tempFileName === this.fileName;
103126
return reloaded;
104127
}
@@ -118,6 +141,23 @@ namespace ts.server {
118141
this.pendingReloadFromDisk = true;
119142
}
120143

144+
/**
145+
* For telemetry purposes, we would like to be able to report the size of the file.
146+
* However, we do not want telemetry to require extra file I/O so we report a size
147+
* that may be stale (e.g. may not reflect change made on disk since the last reload).
148+
* NB: Will read from disk if the file contents have never been loaded because
149+
* telemetry falsely indicating size 0 would be counter-productive.
150+
*/
151+
public getTelemetryFileSize(): number {
152+
return !!this.fileSize
153+
? this.fileSize
154+
: !!this.text // Check text before svc because its length is cheaper
155+
? this.text.length // Could be wrong if this.pendingReloadFromDisk
156+
: !!this.svc
157+
? this.svc.getSnapshot().getLength() // Could be wrong if this.pendingReloadFromDisk
158+
: this.getSnapshot().getLength(); // Should be strictly correct
159+
}
160+
121161
public getSnapshot(): IScriptSnapshot {
122162
return this.useScriptVersionCacheIfValidOrOpen()
123163
? this.svc!.getSnapshot()
@@ -161,7 +201,7 @@ namespace ts.server {
161201
return this.svc!.positionToLineOffset(position);
162202
}
163203

164-
private getFileText(tempFileName?: string) {
204+
private getFileTextAndSize(tempFileName?: string): { text: string, fileSize?: number } {
165205
let text: string;
166206
const fileName = tempFileName || this.fileName;
167207
const getText = () => text === undefined ? (text = this.host.readFile(fileName) || "") : text;
@@ -173,10 +213,10 @@ namespace ts.server {
173213
const service = this.info.containingProjects[0].projectService;
174214
service.logger.info(`Skipped loading contents of large file ${fileName} for info ${this.info.fileName}: fileSize: ${fileSize}`);
175215
this.info.containingProjects[0].projectService.sendLargeFileReferencedEvent(fileName, fileSize);
176-
return "";
216+
return { text: "", fileSize };
177217
}
178218
}
179-
return getText();
219+
return { text: getText() };
180220
}
181221

182222
private switchToScriptVersionCache(): ScriptVersionCache {

src/testRunner/unittests/textStorage.ts

Lines changed: 86 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ namespace ts.textStorage {
1818
const ts2 = new server.TextStorage(host, server.asNormalizedPath(f.path), /*initialVersion*/ undefined, /*info*/undefined!);
1919

2020
ts1.useScriptVersionCache_TestOnly();
21-
ts2.useText();
21+
ts2.useText_TestOnly();
2222

2323
const lineMap = computeLineStarts(f.content);
2424

@@ -29,20 +29,20 @@ namespace ts.textStorage {
2929
for (let offset = 0; offset < end - start; offset++) {
3030
const pos1 = ts1.lineOffsetToPosition(line + 1, offset + 1);
3131
const pos2 = ts2.lineOffsetToPosition(line + 1, offset + 1);
32-
assert.isTrue(pos1 === pos2, `lineOffsetToPosition ${line + 1}-${offset + 1}: expected ${pos1} to equal ${pos2}`);
32+
assert.strictEqual(pos1, pos2, `lineOffsetToPosition ${line + 1}-${offset + 1}: expected ${pos1} to equal ${pos2}`);
3333
}
3434

3535
const {start: start1, length: length1 } = ts1.lineToTextSpan(line);
3636
const {start: start2, length: length2 } = ts2.lineToTextSpan(line);
37-
assert.isTrue(start1 === start2, `lineToTextSpan ${line}::start:: expected ${start1} to equal ${start2}`);
38-
assert.isTrue(length1 === length2, `lineToTextSpan ${line}::length:: expected ${length1} to equal ${length2}`);
37+
assert.strictEqual(start1, start2, `lineToTextSpan ${line}::start:: expected ${start1} to equal ${start2}`);
38+
assert.strictEqual(length1, length2, `lineToTextSpan ${line}::length:: expected ${length1} to equal ${length2}`);
3939
}
4040

4141
for (let pos = 0; pos < f.content.length; pos++) {
4242
const { line: line1, offset: offset1 } = ts1.positionToLineOffset(pos);
4343
const { line: line2, offset: offset2 } = ts2.positionToLineOffset(pos);
44-
assert.isTrue(line1 === line2, `positionToLineOffset ${pos}::line:: expected ${line1} to equal ${line2}`);
45-
assert.isTrue(offset1 === offset2, `positionToLineOffset ${pos}::offset:: expected ${offset1} to equal ${offset2}`);
44+
assert.strictEqual(line1, line2, `positionToLineOffset ${pos}::line:: expected ${line1} to equal ${line2}`);
45+
assert.strictEqual(offset1, offset2, `positionToLineOffset ${pos}::offset:: expected ${offset1} to equal ${offset2}`);
4646
}
4747
});
4848

@@ -52,16 +52,93 @@ namespace ts.textStorage {
5252
const ts1 = new server.TextStorage(host, server.asNormalizedPath(f.path), /*initialVersion*/ undefined, /*info*/undefined!);
5353

5454
ts1.getSnapshot();
55-
assert.isTrue(!ts1.hasScriptVersionCache_TestOnly(), "should not have script version cache - 1");
55+
assert.isFalse(ts1.hasScriptVersionCache_TestOnly(), "should not have script version cache - 1");
5656

5757
ts1.edit(0, 5, " ");
5858
assert.isTrue(ts1.hasScriptVersionCache_TestOnly(), "have script version cache - 1");
5959

60-
ts1.useText();
61-
assert.isTrue(!ts1.hasScriptVersionCache_TestOnly(), "should not have script version cache - 2");
60+
ts1.useText_TestOnly();
61+
assert.isFalse(ts1.hasScriptVersionCache_TestOnly(), "should not have script version cache - 2");
6262

6363
ts1.getLineInfo(0);
6464
assert.isTrue(ts1.hasScriptVersionCache_TestOnly(), "have script version cache - 2");
6565
});
66+
67+
it("should be able to return the file size immediately after construction", () => {
68+
const host = projectSystem.createServerHost([f]);
69+
// Since script info is not used in these tests, just cheat by passing undefined
70+
const ts1 = new server.TextStorage(host, server.asNormalizedPath(f.path), /*initialVersion*/ undefined, /*info*/undefined!);
71+
72+
assert.strictEqual(f.content.length, ts1.getTelemetryFileSize());
73+
});
74+
75+
it("should be able to return the file size when backed by text", () => {
76+
const host = projectSystem.createServerHost([f]);
77+
// Since script info is not used in these tests, just cheat by passing undefined
78+
const ts1 = new server.TextStorage(host, server.asNormalizedPath(f.path), /*initialVersion*/ undefined, /*info*/undefined!);
79+
80+
ts1.useText_TestOnly(f.content);
81+
assert.isFalse(ts1.hasScriptVersionCache_TestOnly());
82+
83+
assert.strictEqual(f.content.length, ts1.getTelemetryFileSize());
84+
});
85+
86+
it("should be able to return the file size when backed by a script version cache", () => {
87+
const host = projectSystem.createServerHost([f]);
88+
// Since script info is not used in these tests, just cheat by passing undefined
89+
const ts1 = new server.TextStorage(host, server.asNormalizedPath(f.path), /*initialVersion*/ undefined, /*info*/undefined!);
90+
91+
ts1.useScriptVersionCache_TestOnly();
92+
assert.isTrue(ts1.hasScriptVersionCache_TestOnly());
93+
94+
assert.strictEqual(f.content.length, ts1.getTelemetryFileSize());
95+
});
96+
97+
it("should be able to return the file size when a JS file is too large to load into text", () => {
98+
const largeFile = {
99+
path: "/a/large.js",
100+
content: " ".repeat(server.maxFileSize + 1)
101+
};
102+
103+
const host = projectSystem.createServerHost([largeFile]);
104+
105+
// The large-file handling requires a ScriptInfo with a containing project
106+
const projectService = projectSystem.createProjectService(host);
107+
projectService.openClientFile(largeFile.path);
108+
const scriptInfo = projectService.getScriptInfo(largeFile.path);
109+
110+
const ts1 = new server.TextStorage(host, server.asNormalizedPath(largeFile.path), /*initialVersion*/ undefined, scriptInfo!);
111+
112+
assert.isTrue(ts1.reloadFromDisk());
113+
assert.isFalse(ts1.hasScriptVersionCache_TestOnly());
114+
115+
assert.strictEqual(largeFile.content.length, ts1.getTelemetryFileSize());
116+
});
117+
118+
it("should return the file size without reloading the file", () => {
119+
const oldText = "hello";
120+
const newText = "goodbye";
121+
122+
const changingFile = {
123+
path: "/a/changing.ts",
124+
content: oldText
125+
};
126+
127+
const host = projectSystem.createServerHost([changingFile]);
128+
// Since script info is not used in these tests, just cheat by passing undefined
129+
const ts1 = new server.TextStorage(host, server.asNormalizedPath(changingFile.path), /*initialVersion*/ undefined, /*info*/undefined!);
130+
131+
assert.isTrue(ts1.reloadFromDisk());
132+
133+
// Refresh the file and notify TextStorage
134+
host.writeFile(changingFile.path, newText);
135+
ts1.delayReloadFromFileIntoText();
136+
137+
assert.strictEqual(oldText.length, ts1.getTelemetryFileSize());
138+
139+
assert.isTrue(ts1.reloadWithFileText());
140+
141+
assert.strictEqual(newText.length, ts1.getTelemetryFileSize());
142+
});
66143
});
67144
}

0 commit comments

Comments
 (0)