Skip to content

Commit 49857b9

Browse files
Improve CLI Responsiveness For Large Tasks (#20)
* Update implementation to use a single data structure for context * Support updating CLI for long compression * update per completion * Update packages and configure timeout for long tests * support for non tty terminals * Missing exit * tty results * reenable long test
1 parent a50daec commit 49857b9

File tree

42 files changed

+252442
-502
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

42 files changed

+252442
-502
lines changed

package.json

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
},
1919
"scripts": {
2020
"pretest": "tsc -p test/tsconfig.json",
21-
"test": "ava -v",
21+
"test": "ava -v --timeout=2m",
2222
"clean": "rimraf dist output test/output",
2323
"build": "rollup -c; shx chmod a+x dist/filesize",
2424
"try": "./dist/filesize",
@@ -32,14 +32,14 @@
3232
"mri": "1.1.4"
3333
},
3434
"devDependencies": {
35-
"@ampproject/rollup-plugin-closure-compiler": "0.20.0",
35+
"@ampproject/rollup-plugin-closure-compiler": "0.21.0",
3636
"@rollup/plugin-commonjs": "11.0.1",
3737
"@rollup/plugin-node-resolve": "7.0.0",
3838
"@rollup/plugin-typescript": "3.0.0",
3939
"@types/bytes": "3.1.0",
4040
"@types/mri": "1.1.0",
4141
"@types/node": "12.12.26",
42-
"ava": "3.1.0",
42+
"ava": "3.2.0",
4343
"husky": "4.2.1",
4444
"lint-staged": "10.0.7",
4545
"magic-string": "0.25.6",
@@ -57,6 +57,7 @@
5757
"yarn": "1.21.1"
5858
},
5959
"filesize": {
60+
"track": ["./dist/**/*.mjs", "./dist/filesize"],
6061
"./dist/filesize": {
6162
"brotli": "3 kB",
6263
"gzip": "3.8 kB",

rollup.config.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ function makeExecutable() {
3333
};
3434
}
3535

36-
const external = ['os', 'zlib', 'path', 'fs', 'stream', 'util', 'events', 'fast-glob'];
36+
const external = ['os', 'zlib', 'path', 'fs', 'stream', 'util', 'events', 'fast-glob', 'process'];
3737
const plugins = executable => [
3838
resolve({ preferBuiltins: true }),
3939
commonjs({ include: 'node_modules/**' }),

src/api.ts

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,17 +16,21 @@
1616

1717
import Project from './validation/Project';
1818
import Config from './validation/Config';
19-
import { Context, ItemConfig, CompressionMap } from './validation/Condition';
19+
import { Context, SizeMap } from './validation/Condition';
2020
import compress from './compress';
2121

22-
export async function report(project: string): Promise<Map<ItemConfig['path'], CompressionMap>> {
22+
export async function report(projectPath: string): Promise<[SizeMap, SizeMap]> {
2323
const conditions = [Project, Config];
2424
let context: Context = {
25-
project,
26-
package: '',
27-
config: [],
28-
track: [],
25+
projectPath,
26+
packagePath: '',
27+
packageContent: '',
2928
silent: true,
29+
originalPaths: new Map(),
30+
// Stores the result of compression <path, [...results]>
31+
compressed: new Map(),
32+
// Stores the basis of comparison.
33+
comparison: new Map(),
3034
};
3135

3236
for (const condition of conditions) {
@@ -36,6 +40,6 @@ export async function report(project: string): Promise<Map<ItemConfig['path'], C
3640
}
3741
}
3842

39-
const { 1: report } = await compress(context);
40-
return report;
43+
await compress(context);
44+
return [context.compressed, context.comparison];
4145
}

src/compress.ts

Lines changed: 76 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,14 @@
1414
* limitations under the License.
1515
*/
1616

17-
import { Context, ItemConfig, Compression, OrderedCompressionMap, CompressionMap } from './validation/Condition';
17+
import { Context, Compression, OrderedCompressionValues, maxSize } from './validation/Condition';
1818
import { cpus } from 'os';
1919
import { constants as brotliConstants, brotliCompress, gzip, ZlibOptions } from 'zlib';
20-
import { readFile } from './fs';
21-
import { LogError, LogReport } from './log';
20+
import { readFile } from './helpers/fs';
21+
import { LogError } from './log/helpers/error';
22+
import { Report } from './log/report';
23+
import { TTYReport } from './log/tty-report';
24+
import { stdout } from 'process';
2225

2326
const COMPRESSION_CONCURRENCY = cpus().length;
2427
const BROTLI_OPTIONS = {
@@ -31,81 +34,105 @@ const BROTLI_OPTIONS = {
3134
const GZIP_OPTIONS: ZlibOptions = {
3235
level: 9,
3336
};
34-
const reported: Map<ItemConfig['path'], CompressionMap> = new Map();
3537

36-
/**
37-
* Pre-populate the report map
38-
* @param configurations
39-
*/
40-
function initReport(configurations: Context['config']) {
41-
for (const config of configurations) {
42-
if (!reported.get(config.originalPath)) {
43-
reported.set(config.originalPath, new Map(OrderedCompressionMap));
44-
}
45-
}
38+
interface CompressionItem {
39+
path: string;
40+
compression: Compression;
41+
maxSize: maxSize;
4642
}
4743

4844
/**
4945
* Use the given configuration and actual size to report item filesize.
46+
* @param report Optional reporter to update with this value
5047
* @param item Configuration for an Item
5148
* @param error Error from compressing an Item
5249
* @param size actual size for this comparison
5350
*/
54-
function store(item: ItemConfig, error: Error | null, size: number): boolean {
51+
function store(report: Report | null, context: Context, item: CompressionItem, error: Error | null, size: number): boolean {
5552
if (error !== null) {
5653
LogError(`Could not compress '${item.path}' with '${item.compression}'.`);
5754
return false;
5855
}
5956

60-
let compressionMap: CompressionMap | undefined;
61-
if ((compressionMap = reported.get(item.originalPath))) {
62-
compressionMap.set(item.compression, [size, item.maxSize]);
57+
// Store the size of the item in the compression map.
58+
const sizeMap = context.compressed.get(item.path);
59+
if (sizeMap === undefined) {
60+
LogError(`Could not find item '${item.path}' with '${item.compression}' in compression map.`);
61+
return false;
62+
}
63+
sizeMap[OrderedCompressionValues.indexOf(item.compression)][0] = size;
64+
65+
report?.update(context);
66+
if (item.maxSize === undefined) {
67+
return true;
6368
}
6469
return size < item.maxSize;
6570
}
6671

6772
/**
68-
* Compress an Item and report status to the console.
69-
* @param item Configuration for an Item.
73+
* Given a context, compress all Items within splitting work eagly per cpu core to achieve some concurrency.
74+
* @param context Finalized Valid Context from Configuration
7075
*/
71-
async function compressor(item: ItemConfig): Promise<boolean> {
72-
const contents = await readFile(item.path);
73-
if (contents) {
74-
const buffer = Buffer.from(contents, 'utf8');
76+
export default async function compress(context: Context): Promise<boolean> {
77+
/**
78+
* Compress an Item and report status to the console.
79+
* @param item Configuration for an Item.
80+
*/
81+
async function compressor(item: CompressionItem): Promise<boolean> {
82+
const contents = await readFile(item.path);
83+
if (contents) {
84+
const buffer = Buffer.from(contents, 'utf8');
7585

76-
switch (item.compression) {
77-
case Compression.BROTLI:
78-
return new Promise(resolve =>
79-
brotliCompress(buffer, BROTLI_OPTIONS, (error: Error | null, result: Buffer) => resolve(store(item, error, result.byteLength))),
80-
);
81-
case Compression.GZIP:
82-
return new Promise(resolve =>
83-
gzip(buffer, GZIP_OPTIONS, (error: Error | null, result: Buffer) => resolve(store(item, error, result.byteLength))),
84-
);
85-
case Compression.NONE:
86-
default:
87-
return store(item, null, buffer.byteLength);
86+
switch (item.compression) {
87+
case 'brotli':
88+
return new Promise(resolve =>
89+
brotliCompress(buffer, BROTLI_OPTIONS, (error: Error | null, result: Buffer) =>
90+
resolve(store(report, context, item, error, result.byteLength)),
91+
),
92+
);
93+
case 'gzip':
94+
return new Promise(resolve =>
95+
gzip(buffer, GZIP_OPTIONS, (error: Error | null, result: Buffer) => resolve(store(report, context, item, error, result.byteLength))),
96+
);
97+
default:
98+
return store(report, context, item, null, buffer.byteLength);
99+
}
88100
}
89-
}
90101

91-
return false;
92-
}
102+
return false;
103+
}
93104

94-
/**
95-
* Given a context, compress all Items within splitting work eagly per cpu core to achieve some concurrency.
96-
* @param context Finalized Valid Context from Configuration
97-
*/
98-
export default async function compress(context: Context): Promise<[boolean, Map<ItemConfig['path'], CompressionMap>]> {
99-
initReport(context.config);
105+
let report: Report | null = null;
106+
const toCompress: Array<CompressionItem> = [];
107+
for (const [path, sizeMapValue] of context.compressed) {
108+
for (let iterator: number = 0; iterator < OrderedCompressionValues.length; iterator++) {
109+
const compression: Compression = OrderedCompressionValues[iterator] as Compression;
110+
const [size, maxSize] = sizeMapValue[iterator];
111+
if (compression === 'none') {
112+
await compressor({ path, compression, maxSize });
113+
}
114+
if (size !== undefined) {
115+
toCompress.push({
116+
path,
117+
compression,
118+
maxSize,
119+
});
120+
}
121+
}
122+
}
100123

124+
report = stdout.isTTY ? new TTYReport(context) : new Report(context);
101125
let success: boolean = true;
102-
for (let iterator: number = 0; iterator < context.config.length; iterator += COMPRESSION_CONCURRENCY) {
103-
let itemsSuccessful = await Promise.all(context.config.slice(iterator, iterator + COMPRESSION_CONCURRENCY).map(compressor));
126+
for (let iterator: number = 0; iterator < toCompress.length; iterator += COMPRESSION_CONCURRENCY) {
127+
if (iterator === 0) {
128+
report.update(context);
129+
}
130+
let itemsSuccessful = await Promise.all(toCompress.slice(iterator, iterator + COMPRESSION_CONCURRENCY).map(compressor));
104131
if (itemsSuccessful.includes(false)) {
105132
success = false;
106133
}
107134
}
108135

109-
LogReport(context, reported);
110-
return [success, reported];
136+
report.end();
137+
return success;
111138
}
File renamed without changes.
File renamed without changes.

src/helpers/process.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/**
2+
* Copyright 2019 The AMP HTML Authors. All Rights Reserved.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS-IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
import { exit } from 'process';
18+
import { exhaust } from '../log/helpers/output';
19+
20+
export function shutdown(code: number) {
21+
exhaust(() => {
22+
exit(code);
23+
});
24+
}

src/index.ts

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -19,36 +19,45 @@ import Project from './validation/Project';
1919
import Config from './validation/Config';
2020
import { Context } from './validation/Condition';
2121
import compress from './compress';
22+
import { LogError } from './log/helpers/error';
23+
import { shutdown } from './helpers/process';
2224

2325
const args = mri(process.argv.slice(2), {
2426
alias: { p: 'project' },
25-
default: { project: process.cwd() },
27+
default: { project: process.cwd(), silent: false },
2628
});
2729

2830
/**
2931
* Read the configuration from the specified project, validate it, perform requested compression, and report the results.
3032
*/
3133
(async function() {
32-
const { project } = args;
34+
const { project: projectPath, silent } = args;
3335
const conditions = [Project, Config];
34-
let context: Context = {
35-
project,
36-
package: '',
37-
config: [],
38-
track: [],
39-
silent: false,
36+
const context: Context = {
37+
projectPath,
38+
packagePath: '',
39+
packageContent: '',
40+
silent,
41+
originalPaths: new Map(),
42+
// Stores the result of compression <path, [...results]>
43+
compressed: new Map(),
44+
// Stores the basis of comparison.
45+
comparison: new Map(),
4046
};
4147

48+
let errorOccured: boolean = false;
4249
for (const condition of conditions) {
4350
const message = await condition(context)();
4451
if (message !== null) {
45-
console.log(message);
46-
process.exit(5);
52+
LogError(message);
53+
shutdown(5);
54+
errorOccured = true;
4755
}
4856
}
4957

50-
const [compressionSuccess] = await compress(context);
51-
if (!compressionSuccess) {
52-
process.exit(6);
58+
if (!errorOccured) {
59+
if (!(await compress(context))) {
60+
shutdown(6);
61+
}
5362
}
5463
})();

0 commit comments

Comments
 (0)