Skip to content

Commit

Permalink
[merge-conflicts] remove line splitting, and improve performance
Browse files Browse the repository at this point in the history 
* add `lodash.throttle`
* batch application of newDecorations
* avoid `Array.map` and `Array.find` calls, which showed hotspots
* use precompiled regex' in parser

Signed-off-by: Alex Tugarev <[email protected]>
  • Loading branch information
@AlexTugarev
AlexTugarev committed Mar 15, 2018
1 parent 209d85c commit bb02ba4
Show file tree
Hide file tree
Showing 10 changed files with 233 additions and 159 deletions.
2 changes: 2 additions & 0 deletions packages/core/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
"@types/perfect-scrollbar": "^1.3.0",
"@types/ws": "^3.0.2",
"@types/yargs": "^8.0.2",
"@types/lodash.throttle": "^4.1.3",
"lodash.throttle": "^4.1.1",
"ajv": "^5.2.2",
"body-parser": "^1.17.2",
"bunyan": "^1.8.10",
Expand Down
19 changes: 10 additions & 9 deletions packages/merge-conflicts/src/browser/merge-conflicts-code-lense-provider.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,23 +5,24 @@
* You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0
*/

import { injectable, inject } from "inversify";
import { inject, injectable } from 'inversify';
import { CodeLensProvider, CodeLensParams, CodeLens, CancellationToken } from '@theia/languages/lib/common';
import { MergeConflict, MergeConflictsCommands as Commands, MergeConflictCommandArgument } from "./merge-conflict";
import { MergeConflictsService } from "./merge-conflicts-service";
import { MergeConflictsProvider } from './merge-conflicts-provider';
import { MergeConflict, MergeConflictsCommands as Commands, MergeConflictCommandArgument } from './merge-conflict';

@injectable()
export class MergeConflictsCodeLensProvider implements CodeLensProvider {

constructor(
@inject(MergeConflictsService) protected readonly mergeConflictsService: MergeConflictsService,
) { }
@inject(MergeConflictsProvider)
protected readonly mergeConflictsProvider: MergeConflictsProvider;

provideCodeLenses(params: CodeLensParams, token: CancellationToken): Promise<CodeLens[]> {
async provideCodeLenses(params: CodeLensParams, token: CancellationToken): Promise<CodeLens[]> {
const uri = params.textDocument.uri;
const mergeConflicts: MergeConflict[] = this.mergeConflictsService.get(uri);
const mergeConflicts = await this.mergeConflictsProvider.get(uri);
const result: CodeLens[] = [];
mergeConflicts.forEach(mergeConflict => result.push(...this.toCodeLense(uri, mergeConflict)));
if (mergeConflicts) {
mergeConflicts.mergeConflicts.forEach(mergeConflict => result.push(...this.toCodeLense(uri, mergeConflict)));
}
return Promise.resolve(result);
}

Expand Down
75 changes: 32 additions & 43 deletions packages/merge-conflicts/src/browser/merge-conflicts-decorations.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
*/

import { injectable, inject } from "inversify";
import { EditorDecorationsService, OverviewRulerLane, Range, EditorDecoration, EditorDecorationOptions } from "@theia/editor/lib/browser";
import { MergeConflictUpdateParams } from "./merge-conflicts-service";
import { EditorDecorationsService, OverviewRulerLane, EditorDecoration, EditorDecorationOptions } from "@theia/editor/lib/browser";
import { MergeConflicts } from "./merge-conflicts-provider";

@injectable()
export class MergeConflictsDecorations {
Expand All @@ -16,79 +16,68 @@ export class MergeConflictsDecorations {
@inject(EditorDecorationsService) protected readonly decorationsService: EditorDecorationsService,
) { }

onMergeConflictUpdate(params: MergeConflictUpdateParams): void {
decorate(params: MergeConflicts): void {
const uri = params.uri;
const mergeConflicts = params.mergeConflicts;
this.setDecorations(uri, MergeConflictsDecorations.Kind.CurrentMarker, mergeConflicts.map(c => c.current.marker!));
this.setDecorations(uri, MergeConflictsDecorations.Kind.CurrentContent, mergeConflicts.map(c => c.current.content!));
this.setDecorations(uri, MergeConflictsDecorations.Kind.IncomingMarker, mergeConflicts.map(c => c.incoming.marker!));
this.setDecorations(uri, MergeConflictsDecorations.Kind.IncomingContent, mergeConflicts.map(c => c.incoming.content!));

const baseMarkerRanges: Range[] = [];
const baseContentRanges: Range[] = [];
mergeConflicts.forEach(c => c.bases.forEach(b => {
if (b.marker) {
baseMarkerRanges.push(b.marker);
}
if (b.content) {
baseContentRanges.push(b.content);
const newDecorations: EditorDecoration[] = [];
for (const mergeConflict of mergeConflicts) {
newDecorations.push({ range: mergeConflict.current.marker!, options: MergeConflictsDecorations.Options.CurrentMarker });
newDecorations.push({ range: mergeConflict.current.content!, options: MergeConflictsDecorations.Options.CurrentContent });
newDecorations.push({ range: mergeConflict.incoming.marker!, options: MergeConflictsDecorations.Options.IncomingMarker });
newDecorations.push({ range: mergeConflict.incoming.content!, options: MergeConflictsDecorations.Options.IncomingContent });
for (const base of mergeConflict.bases) {
if (base.marker) {
newDecorations.push({ range: base.marker, options: MergeConflictsDecorations.Options.BaseMarker });
}
if (base.content) {
newDecorations.push({ range: base.content, options: MergeConflictsDecorations.Options.BaseContent });
}
}
}));
this.setDecorations(uri, MergeConflictsDecorations.Kind.BaseMarker, baseMarkerRanges);
this.setDecorations(uri, MergeConflictsDecorations.Kind.BaseContent, baseContentRanges);
}
this.setDecorations(uri, newDecorations);
}

protected setDecorations(uri: string, kind: MergeConflictsDecorations.Kind, ranges: Range[]) {
const options = MergeConflictsDecorations.Options[kind];
const newDecorations = ranges.map(range => <EditorDecoration>{ range, options });
protected setDecorations(uri: string, newDecorations: EditorDecoration[]) {
const kind = 'merge-conflicts';
this.decorationsService.setDecorations({ uri, kind, newDecorations });
}

}

export namespace MergeConflictsDecorations {

export enum Kind {
CurrentMarker = 'merge-conflict-current-marker',
CurrentContent = 'merge-conflict-current-content',
BaseMarker = 'merge-conflict-base-marker',
BaseContent = 'merge-conflict-base-content',
IncomingMarker = 'merge-conflict-incoming-marker',
IncomingContent = 'merge-conflict-incoming-content',
}

export const Options = {
[Kind.CurrentMarker]: <EditorDecorationOptions>{
CurrentMarker: <EditorDecorationOptions>{
isWholeLine: true,
className: Kind.CurrentMarker.toString()
className: 'merge-conflict-current-marker'
},
[Kind.CurrentContent]: <EditorDecorationOptions>{
CurrentContent: <EditorDecorationOptions>{
isWholeLine: true,
className: Kind.CurrentContent.toString(),
className: 'merge-conflict-current-content',
overviewRuler: {
position: OverviewRulerLane.Full,
color: 'rgba(0, 255, 0, 0.3)',
}
},
[Kind.BaseMarker]: <EditorDecorationOptions>{
BaseMarker: <EditorDecorationOptions>{
isWholeLine: true,
className: Kind.BaseMarker.toString()
className: 'merge-conflict-base-marker'
},
[Kind.BaseContent]: <EditorDecorationOptions>{
BaseContent: <EditorDecorationOptions>{
isWholeLine: true,
className: Kind.BaseContent.toString(),
className: 'merge-conflict-base-content',
overviewRuler: {
position: OverviewRulerLane.Full,
color: 'rgba(125, 125, 125, 0.3)',
}
},
[Kind.IncomingMarker]: <EditorDecorationOptions>{
IncomingMarker: <EditorDecorationOptions>{
isWholeLine: true,
className: Kind.IncomingMarker.toString()
className: 'merge-conflict-incoming-marker'
},
[Kind.IncomingContent]: <EditorDecorationOptions>{
IncomingContent: <EditorDecorationOptions>{
isWholeLine: true,
className: Kind.IncomingContent.toString(),
className: 'merge-conflict-incoming-content',
overviewRuler: {
position: OverviewRulerLane.Full,
color: 'rgba(0, 0, 255, 0.3)',
Expand Down
38 changes: 17 additions & 21 deletions packages/merge-conflicts/src/browser/merge-conflicts-frontend-contribution.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,40 +8,36 @@
import { injectable, inject } from "inversify";
import { FrontendApplication, FrontendApplicationContribution } from '@theia/core/lib/browser';
import { CommandContribution, CommandRegistry } from '@theia/core/lib/common';
import { Languages, Workspace } from '@theia/languages/lib/common';
import { Languages } from '@theia/languages/lib/common';
import { MergeConflictsCodeLensProvider } from './merge-conflicts-code-lense-provider';
import { MergeConflictResolver } from './merge-conflict-resolver';
import { MergeConflictsCommands as Commands } from './merge-conflict';
import { MergeConflictsService } from "./merge-conflicts-service";
import { MergeConflictsProvider } from "./merge-conflicts-provider";
import { MergeConflictsDecorations } from "./merge-conflicts-decorations";

@injectable()
export class MergeConflictsFrontendContribution implements FrontendApplicationContribution, CommandContribution {

constructor(
@inject(Languages) protected readonly languages: Languages,
@inject(MergeConflictsCodeLensProvider) protected readonly mergeConflictsCodeLensProvider: MergeConflictsCodeLensProvider,
@inject(MergeConflictResolver) protected readonly mergeConflictResolver: MergeConflictResolver,
@inject(MergeConflictsDecorations) protected readonly mergeConflictsDecorations: MergeConflictsDecorations,
@inject(MergeConflictsService) protected readonly mergeConflictsService: MergeConflictsService,
@inject(Workspace) protected readonly workspace: Workspace,
) { }
@inject(Languages)
protected readonly languages: Languages;

@inject(MergeConflictsCodeLensProvider)
protected readonly mergeConflictsCodeLensProvider: MergeConflictsCodeLensProvider;

@inject(MergeConflictResolver)
protected readonly mergeConflictResolver: MergeConflictResolver;

@inject(MergeConflictsDecorations)
protected readonly decorator: MergeConflictsDecorations;

@inject(MergeConflictsProvider)
protected readonly mergeConflictsProvider: MergeConflictsProvider;

onStart(app: FrontendApplication): void {
if (this.languages.registerCodeLensProvider) {
this.languages.registerCodeLensProvider([{ pattern: '**/*' }], this.mergeConflictsCodeLensProvider);
}
this.workspace.onDidOpenTextDocument(document => {
window.setTimeout(() => {
this.updateEditorDecorations(document.uri);
}, 1);
});
this.workspace.onDidChangeTextDocument(params => this.updateEditorDecorations(params.textDocument.uri));
this.mergeConflictsService.onMergeConflictUpdate(params => this.mergeConflictsDecorations.onMergeConflictUpdate(params));
}

protected updateEditorDecorations(uri: string) {
this.mergeConflictsService.get(uri);
this.mergeConflictsProvider.onDidUpdate(params => this.decorator.decorate(params));
}

registerCommands(registry: CommandRegistry): void {
Expand Down
4 changes: 2 additions & 2 deletions packages/merge-conflicts/src/browser/merge-conflicts-frontend-module.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { MergeConflictsFrontendContribution } from './merge-conflicts-frontend-c
import { MergeConflictsCodeLensProvider } from './merge-conflicts-code-lense-provider';
import { MergeConflictsParser } from './merge-conflicts-parser';
import { MergeConflictResolver } from './merge-conflict-resolver';
import { MergeConflictsService } from './merge-conflicts-service';
import { MergeConflictsProvider } from './merge-conflicts-provider';
import { MergeConflictsDecorations } from './merge-conflicts-decorations';

import '../../src/browser/style/index.css';
Expand All @@ -23,7 +23,7 @@ export default new ContainerModule(bind => {
bind(MergeConflictsCodeLensProvider).toSelf().inSingletonScope();
bind(MergeConflictsDecorations).toSelf().inSingletonScope();
bind(MergeConflictsFrontendContribution).toSelf().inSingletonScope();
bind(MergeConflictsService).toSelf().inSingletonScope();
bind(MergeConflictsProvider).toSelf().inSingletonScope();
[CommandContribution, FrontendApplicationContribution].forEach(serviceIdentifier =>
bind(serviceIdentifier).toDynamicValue(c => c.container.get(MergeConflictsFrontendContribution)).inSingletonScope()
);
Expand Down
28 changes: 19 additions & 9 deletions packages/merge-conflicts/src/browser/merge-conflicts-parser.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ chai.use(require('chai-string'));

import { MergeConflictsParser } from './merge-conflicts-parser';
import { Range, Position } from '@theia/editor/lib/browser';
import { MergeConflict } from './merge-conflict';

let parser: MergeConflictsParser;

Expand All @@ -20,10 +21,19 @@ before(() => {

// tslint:disable:no-unused-expression

function parse(contents: string): MergeConflict[] {
const splitted = contents.split('\n');
const input = <MergeConflictsParser.Input>{
lineCount: splitted.length,
getLine: lineNumber => splitted[lineNumber],
};
return parser.parse(input);
}

describe("merge-conflict-parser", () => {

it("simple merge conflict", () => {
const conflicts = parser.parse(
const conflicts = parse(
`<<<<<<< HEAD
foo changed on master
bar changed on master
Expand Down Expand Up @@ -55,7 +65,7 @@ foo on branch
bar on branch
>>>>>>> branch
last line`;
const conflicts = parser.parse(content);
const conflicts = parse(content);
expect(conflicts).to.have.lengthOf(1);
const conflict = conflicts[0];

Expand Down Expand Up @@ -101,7 +111,7 @@ bar on branch`);
});

it("multiple merge conflicts", () => {
const conflicts = parser.parse(
const conflicts = parse(
`<<<<<<< HEAD
foo changed on master
bar changed on master
Expand Down Expand Up @@ -130,7 +140,7 @@ bar on branch
});

it("merge conflict with bases", () => {
const conflicts = parser.parse(
const conflicts = parse(
`<<<<<<< HEAD
foo changed on master
bar changed on master
Expand Down Expand Up @@ -170,7 +180,7 @@ bar on branch
});

it("broken 1: second current marker in current content", () => {
const conflicts = parser.parse(
const conflicts = parse(
`<<<<<<< HEAD
<<<<<<< HEAD
foo changed on master
Expand All @@ -193,7 +203,7 @@ bar on branch
});

it("broken 2: current marker in incoming content", () => {
const conflicts = parser.parse(
const conflicts = parse(
`<<<<<<< HEAD
foo changed on master
bar changed on master
Expand All @@ -208,7 +218,7 @@ bar on branch
});

it("broken 3: second separator", () => {
const conflicts = parser.parse(
const conflicts = parse(
`<<<<<<< HEAD
foo changed on master
bar changed on master
Expand All @@ -223,7 +233,7 @@ bar on branch
});

it("broken 4: second separator in incoming content", () => {
const conflicts = parser.parse(
const conflicts = parse(
`<<<<<<< HEAD
foo changed on master
bar changed on master
Expand All @@ -238,7 +248,7 @@ bar on branch
});

it("broken 5: incoming marker, no separator", () => {
const conflicts = parser.parse(
const conflicts = parse(
`<<<<<<< HEAD
foo changed on master
bar changed on master
Expand Down
Loading

0 comments on commit bb02ba4

Please sign in to comment.