Skip to content

Commit

Permalink
Improve cache handling on the frontend, cache executions on the backe…
Browse files Browse the repository at this point in the history
…nd, and improve controls on the exec pane (#5111)
  • Loading branch information
jeremy-rifkin committed Jun 11, 2023
1 parent 910d69f commit 60ce06b
Show file tree
Hide file tree
Showing 24 changed files with 274 additions and 180 deletions.
2 changes: 1 addition & 1 deletion .eslintrc.yml
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ rules:
import/first: error
import/newline-after-import: error
import/no-absolute-path: error
import/no-cycle: error
#import/no-cycle: error # TODO(jeremy-rifkin) disabled for now due to compilation types
import/no-default-export: error
import/no-deprecated: error
import/no-mutable-exports: error
Expand Down
15 changes: 14 additions & 1 deletion docs/API.md
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,20 @@ The filters are a JSON object with `true`/`false` values. If not supplied, defau
filters override their default values. The `compilerOptions` is used to pass extra arguments to the back end, and is
probably not useful for most REST users.

To force a cache bypass, set `bypassCache` in the root of the request to `true`.
To force a cache bypass, `bypassCache` can be set. This accepts an enum value according to:

```ts
export enum BypassCache {
None = 0,
Compilation = 1,
Execution = 2,
}
```

If bypass compile cache is specified and an execution is to happen, the execution cache will also be bypassed.

Note: `bypassCache` previously accepted a boolean. The enum values have been carefully chosen for backwards
compatibility.

Filters include `binary`, `binaryObject`, `labels`, `intel`, `directives` and `demangle`, which correspond to the UI
buttons on the HTML version.
Expand Down
69 changes: 54 additions & 15 deletions lib/base-compiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,18 @@ import * as PromClient from 'prom-client';
import temp from 'temp';
import _ from 'underscore';

import type {
import {
BufferOkFunc,
BuildResult,
BuildStep,
BypassCache,
CompilationCacheKey,
CompilationInfo,
CompilationResult,
CustomInputForTool,
ExecutionOptions,
bypassCompilationCache,
bypassExecutionCache,
} from '../types/compilation/compilation.interfaces.js';
import type {
LLVMOptPipelineBackendOptions,
Expand Down Expand Up @@ -489,7 +492,7 @@ export class BaseCompiler implements ICompiler {
maxSize: number,
intelAsm,
demangle,
staticReloc: boolean,
staticReloc: boolean | undefined,
dynamicReloc: boolean,
filters: ParseFiltersAndOutputOptions,
) {
Expand Down Expand Up @@ -1718,11 +1721,13 @@ export class BaseCompiler implements ICompiler {
};
}

async getOrBuildExecutable(key) {
async getOrBuildExecutable(key, bypassCache: BypassCache) {
const dirPath = await this.newTempDir();

const buildResults = await this.loadPackageWithExecutable(key, dirPath);
if (buildResults) return buildResults;
if (!bypassCompilationCache(bypassCache)) {
const buildResults = await this.loadPackageWithExecutable(key, dirPath);
if (buildResults) return buildResults;
}

let compilationResult;
try {
Expand Down Expand Up @@ -1843,9 +1848,11 @@ export class BaseCompiler implements ICompiler {
};
}

async handleExecution(key, executeParameters): Promise<CompilationResult> {
if (this.compiler.interpreted) return this.handleInterpreting(key, executeParameters);
const buildResult = await this.getOrBuildExecutable(key);
async doExecution(key, executeParameters, bypassCache: BypassCache): Promise<CompilationResult> {
if (this.compiler.interpreted) {
return this.handleInterpreting(key, executeParameters);
}
const buildResult = await this.getOrBuildExecutable(key, bypassCache);
if (buildResult.code !== 0) {
return {
code: -1,
Expand Down Expand Up @@ -1892,6 +1899,23 @@ export class BaseCompiler implements ICompiler {
};
}

async handleExecution(key, executeParameters, bypassCache: BypassCache): Promise<CompilationResult> {
// stringify now so shallow copying isn't a problem, I think the executeParameters get modified
const execKey = JSON.stringify({key, executeParameters});
if (!bypassExecutionCache(bypassCache)) {
const cacheResult = await this.env.cacheGet(execKey as any);
if (cacheResult) {
return cacheResult;
}
}

const result = await this.doExecution(key, executeParameters, bypassCache);
if (!bypassExecutionCache(bypassCache)) {
await this.env.cachePut(execKey, result, undefined);
}
return result;
}

getCacheKey(source, options, backendOptions, filters, tools, libraries, files) {
return {compiler: this.compiler, source, options, backendOptions, filters, tools, libraries, files};
}
Expand Down Expand Up @@ -2262,7 +2286,7 @@ export class BaseCompiler implements ICompiler {
}
}

async cmake(files, key) {
async cmake(files, key, bypassCache: BypassCache) {
// key = {source, options, backendOptions, filters, bypassCache, tools, executionParameters, libraries};

if (!this.compiler.supportsBinary) {
Expand Down Expand Up @@ -2300,7 +2324,9 @@ export class BaseCompiler implements ICompiler {

const outputFilename = this.getExecutableFilename(path.join(dirPath, 'build'), this.outputFilebase, key);

let fullResult = await this.loadPackageWithExecutable(cacheKey, dirPath);
let fullResult = !bypassExecutionCache(bypassCache)
? await this.loadPackageWithExecutable(cacheKey, dirPath)
: null;
if (fullResult) {
fullResult.fetchedFromCache = true;

Expand Down Expand Up @@ -2421,6 +2447,7 @@ export class BaseCompiler implements ICompiler {
cacheKey.filters,
libsAndOptions.options,
optOutput,
bypassCache,
path.join(dirPath, 'build'),
);

Expand Down Expand Up @@ -2469,7 +2496,17 @@ export class BaseCompiler implements ICompiler {
}
}

async compile(source, options, backendOptions, filters, bypassCache, tools, executionParameters, libraries, files) {
async compile(
source,
options,
backendOptions,
filters,
bypassCache: BypassCache,
tools,
executionParameters,
libraries,
files,
) {
const optionsError = this.checkOptions(options);
if (optionsError) throw optionsError;
const sourceError = this.checkSource(source);
Expand All @@ -2494,7 +2531,7 @@ export class BaseCompiler implements ICompiler {
filters = Object.assign({}, filters);
filters.execute = false;

if (!bypassCache) {
if (!bypassCompilationCache(bypassCache)) {
const cacheRetreiveTimeStart = process.hrtime.bigint();
// TODO: We should be able to eliminate this any cast. `key` should be cacheable (if it's not that's a big
// problem) Because key coantains a CompilerInfo which contains a function member it can't be assigned to a
Expand All @@ -2512,7 +2549,7 @@ export class BaseCompiler implements ICompiler {
result.execResult = await this.env.enqueue(async () => {
const start = performance.now();
executionQueueTimeHistogram.observe((start - queueTime) / 1000);
const res = await this.handleExecution(key, executeParameters);
const res = await this.handleExecution(key, executeParameters, bypassCache);
executionTimeHistogram.observe((performance.now() - start) / 1000);
return res;
});
Expand All @@ -2532,7 +2569,7 @@ export class BaseCompiler implements ICompiler {
source = this.preProcess(source, filters);

if (backendOptions.executorRequest) {
const execResult = await this.handleExecution(key, executeParameters);
const execResult = await this.handleExecution(key, executeParameters, bypassCache);
if (execResult && execResult.buildResult) {
this.doTempfolderCleanup(execResult.buildResult);
}
Expand Down Expand Up @@ -2570,6 +2607,7 @@ export class BaseCompiler implements ICompiler {
filters,
options,
optOutput,
bypassCache,
);
})();
compilationTimeHistogram.observe((performance.now() - start) / 1000);
Expand All @@ -2587,10 +2625,11 @@ export class BaseCompiler implements ICompiler {
filters,
options,
optOutput,
bypassCache: BypassCache,
customBuildPath?,
) {
// Start the execution as soon as we can, but only await it at the end.
const execPromise = doExecute ? this.handleExecution(key, executeParameters) : null;
const execPromise = doExecute ? this.handleExecution(key, executeParameters, bypassCache) : null;

if (result.hasOptOutput) {
delete result.optPath;
Expand Down
3 changes: 2 additions & 1 deletion lib/compilers/java.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import {logger} from '../logger.js';
import * as utils from '../utils.js';

import {JavaParser} from './argument-parsers.js';
import {BypassCache} from '../../types/compilation/compilation.interfaces.js';

export class JavaCompiler extends BaseCompiler {
static get key() {
Expand Down Expand Up @@ -128,7 +129,7 @@ export class JavaCompiler extends BaseCompiler {
}

override async handleInterpreting(key, executeParameters) {
const compileResult = await this.getOrBuildExecutable(key);
const compileResult = await this.getOrBuildExecutable(key, BypassCache.None);
if (compileResult.code === 0) {
executeParameters.args = [
'-Xss136K', // Reduce thread stack size
Expand Down
3 changes: 2 additions & 1 deletion lib/compilers/kotlin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
// ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
// POSSIBILITY OF SUCH DAMAGE.

import {BypassCache} from '../../types/compilation/compilation.interfaces.js';
import type {PreliminaryCompilerInfo} from '../../types/compiler.interfaces.js';
import type {ParseFiltersAndOutputOptions} from '../../types/features/filters.interfaces.js';

Expand Down Expand Up @@ -98,7 +99,7 @@ export class KotlinCompiler extends JavaCompiler {
...key,
options: ['-include-runtime', '-d', 'example.jar'],
};
const compileResult = await this.getOrBuildExecutable(alteredKey);
const compileResult = await this.getOrBuildExecutable(alteredKey, BypassCache.None);
executeParameters.args = [
'-Xss136K', // Reduce thread stack size
'-XX:CICompilerCount=2', // Reduce JIT compilation threads. 2 is minimum
Expand Down
11 changes: 8 additions & 3 deletions lib/compilers/win32-mingw-clang.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,12 @@

import path from 'path';

import {BuildResult, CompilationResult, ExecutionOptions} from '../../types/compilation/compilation.interfaces.js';
import {
BuildResult,
BypassCache,
CompilationResult,
ExecutionOptions,
} from '../../types/compilation/compilation.interfaces.js';
import {ParseFiltersAndOutputOptions} from '../../types/features/filters.interfaces.js';

import {copyNeededDlls} from '../win-utils.js';
Expand Down Expand Up @@ -102,8 +107,8 @@ export class Win32MingWClang extends ClangCompiler {
return result;
}

override async handleExecution(key, executeParameters): Promise<CompilationResult> {
override async handleExecution(key, executeParameters, bypassCache: BypassCache): Promise<CompilationResult> {
const execOptions = this.getDefaultExecOptions();
return super.handleExecution(key, {...executeParameters, env: execOptions.env});
return super.handleExecution(key, {...executeParameters, env: execOptions.env}, bypassCache);
}
}
11 changes: 8 additions & 3 deletions lib/compilers/win32-mingw-gcc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,12 @@

import path from 'path';

import {BuildResult, CompilationResult, ExecutionOptions} from '../../types/compilation/compilation.interfaces.js';
import {
BuildResult,
BypassCache,
CompilationResult,
ExecutionOptions,
} from '../../types/compilation/compilation.interfaces.js';
import {ParseFiltersAndOutputOptions} from '../../types/features/filters.interfaces.js';

import {copyNeededDlls} from '../win-utils.js';
Expand Down Expand Up @@ -102,8 +107,8 @@ export class Win32MingWGcc extends GCCCompiler {
return result;
}

override async handleExecution(key, executeParameters): Promise<CompilationResult> {
override async handleExecution(key, executeParameters, bypassCache: BypassCache): Promise<CompilationResult> {
const execOptions = this.getDefaultExecOptions();
return super.handleExecution(key, {...executeParameters, env: execOptions.env});
return super.handleExecution(key, {...executeParameters, env: execOptions.env}, bypassCache);
}
}
6 changes: 4 additions & 2 deletions lib/handlers/compile.interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
// ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
// POSSIBILITY OF SUCH DAMAGE.

import {BypassCache} from '../../types/compilation/compilation.interfaces.js';

// IF YOU MODIFY ANYTHING HERE PLEASE UPDATE THE DOCUMENTATION!

// This type models a request so all fields must be optional strings.
Expand Down Expand Up @@ -52,12 +54,12 @@ export type CompilationRequestArgs = {
export type CompileRequestJsonBody = {
options: CompilationRequestArgs;
source: string;
bypassCache: boolean;
bypassCache: BypassCache;
};

export type CompileRequestTextBody = {
source: string;
bypassCache: boolean;
bypassCache: BypassCache;
options: any;
userArguments: string;
executeParametersArgs: any;
Expand Down
25 changes: 14 additions & 11 deletions lib/handlers/compile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ import {
} from './compile.interfaces.js';
import {remove} from '../common-utils.js';
import {CompilerOverrideOptions} from '../../types/compilation/compiler-overrides.interfaces.js';
import {BypassCache, CompileChildLibraries, ExecutionParams} from '../../types/compilation/compilation.interfaces.js';
import {SentryCapture} from '../sentry.js';

temp.track();
Expand Down Expand Up @@ -82,20 +83,15 @@ function initialise(compilerEnv: CompilationEnvironment) {
}, tempDirCleanupSecs * 1000);
}

export type ExecutionParams = {
args: string[];
stdin: string;
};

type ParsedRequest = {
source: string;
options: string[];
backendOptions: Record<string, any>;
filters: ParseFiltersAndOutputOptions;
bypassCache: boolean;
bypassCache: BypassCache;
tools: any;
executionParameters: ExecutionParams;
libraries: any[];
libraries: CompileChildLibraries[];
};

export class CompileHandler {
Expand Down Expand Up @@ -353,7 +349,7 @@ export class CompileHandler {
options: string,
backendOptions: Record<string, any> = {},
filters: ParseFiltersAndOutputOptions,
bypassCache = false,
bypassCache = BypassCache.None,
tools;
const execReqParams: ExecutionRequestParams = {};
let libraries: any[] = [];
Expand All @@ -363,7 +359,7 @@ export class CompileHandler {
const jsonRequest = this.checkRequestRequirements(req);
const requestOptions = jsonRequest.options;
source = jsonRequest.source;
if (jsonRequest.bypassCache) bypassCache = true;
if (jsonRequest.bypassCache) bypassCache = jsonRequest.bypassCache;
options = requestOptions.userArguments;
const execParams = requestOptions.executeParameters || {};
execReqParams.args = execParams.args;
Expand All @@ -375,7 +371,7 @@ export class CompileHandler {
} else if (req.body && req.body.compiler) {
const textRequest = req.body as CompileRequestTextBody;
source = textRequest.source;
if (textRequest.bypassCache) bypassCache = true;
if (textRequest.bypassCache) bypassCache = textRequest.bypassCache;
options = textRequest.userArguments;
execReqParams.args = textRequest.executeParametersArgs;
execReqParams.stdin = textRequest.executeParametersStdin;
Expand Down Expand Up @@ -423,6 +419,11 @@ export class CompileHandler {
for (const tool of tools) {
tool.args = utils.splitArguments(tool.args);
}

// Backwards compatibility: bypassCache used to be a boolean.
// Convert a boolean input to an enum's underlying numeric value
bypassCache = 1 * bypassCache;

return {
source,
options: utils.splitArguments(options),
Expand Down Expand Up @@ -497,7 +498,9 @@ export class CompileHandler {
this.cmakeCounter.inc({language: compiler.lang.id});
const options = this.parseRequest(req, compiler);
compiler
.cmake(req.body.files, options)
// Backwards compatibility: bypassCache used to be a boolean.
// Convert a boolean input to an enum's underlying numeric value
.cmake(req.body.files, options, req.body.bypassCache * 1)
.then(result => {
if (result.didExecute || (result.execResult && result.execResult.didExecute))
this.cmakeExecuteCounter.inc({language: compiler.lang.id});
Expand Down
Loading

0 comments on commit 60ce06b

Please sign in to comment.