Merge pull request #3559 from silamon/options3

Resolve perf regression from OptionsService.options

src/common/CoreTerminal.ts

@@ -247,7 +247,7 @@ export abstract class CoreTerminal extends Disposable implements ICoreTerminal {
   }
 
   protected _setup(): void {
-    if (this.optionsService.options.windowsMode) {
+    if (this.optionsService.rawOptions.windowsMode) {
       this._enableWindowsMode();
     }
   }
@@ -267,7 +267,7 @@ export abstract class CoreTerminal extends Disposable implements ICoreTerminal {
         this.buffers.resize(this.cols, this.rows);
         break;
       case 'windowsMode':
-        if (this.optionsService.options.windowsMode) {
+        if (this.optionsService.rawOptions.windowsMode) {
           this._enableWindowsMode();
         } else {
           this._windowsMode?.dispose();

src/common/InputHandler.ts

@@ -177,8 +177,8 @@ class DECRQSS implements IDcsHandler {
         break;
       case ' q': // DECSCUSR
         const STYLES: { [key: string]: number } = { 'block': 2, 'underline': 4, 'bar': 6 };
-        let style = STYLES[this._optionsService.options.cursorStyle];
-        style -= this._optionsService.options.cursorBlink ? 1 : 0;
+        let style = STYLES[this._optionsService.rawOptions.cursorStyle];
+        style -= this._optionsService.rawOptions.cursorBlink ? 1 : 0;
         this._coreService.triggerDataEvent(`${C0.ESC}P1$r${style} q${C0.ESC}\\`);
         break;
       default:
@@ -603,7 +603,7 @@ export class InputHandler extends Disposable implements IInputHandler {
     let code: number;
     let chWidth: number;
     const charset = this._charsetService.charset;
-    const screenReaderMode = this._optionsService.options.screenReaderMode;
+    const screenReaderMode = this._optionsService.rawOptions.screenReaderMode;
     const cols = this._bufferService.cols;
     const wraparoundMode = this._coreService.decPrivateModes.wraparound;
     const insertMode = this._coreService.modes.insertMode;
@@ -745,7 +745,7 @@ export class InputHandler extends Disposable implements IInputHandler {
     if (id.final === 't' && !id.prefix && !id.intermediates) {
       // security: always check whether window option is allowed
       return this._parser.registerCsiHandler(id, params => {
-        if (!paramToWindowOption(params.params[0], this._optionsService.options.windowOptions)) {
+        if (!paramToWindowOption(params.params[0], this._optionsService.rawOptions.windowOptions)) {
           return true;
         }
         return callback(params);
@@ -800,7 +800,7 @@ export class InputHandler extends Disposable implements IInputHandler {
    */
   public lineFeed(): boolean {
     this._dirtyRowService.markDirty(this._activeBuffer.y);
-    if (this._optionsService.options.convertEol) {
+    if (this._optionsService.rawOptions.convertEol) {
       this._activeBuffer.x = 0;
     }
     this._activeBuffer.y++;
@@ -904,7 +904,7 @@ export class InputHandler extends Disposable implements IInputHandler {
     }
     const originalX = this._activeBuffer.x;
     this._activeBuffer.x = this._activeBuffer.nextStop();
-    if (this._optionsService.options.screenReaderMode) {
+    if (this._optionsService.rawOptions.screenReaderMode) {
       this._onA11yTab.fire(this._activeBuffer.x - originalX);
     }
     return true;
@@ -1763,7 +1763,7 @@ export class InputHandler extends Disposable implements IInputHandler {
    * @param term The terminal name to evaluate
    */
   private _is(term: string): boolean {
-    return (this._optionsService.options.termName + '').indexOf(term) === 0;
+    return (this._optionsService.rawOptions.termName + '').indexOf(term) === 0;
   }
 
   /**
@@ -1929,7 +1929,7 @@ export class InputHandler extends Disposable implements IInputHandler {
            * This is only active if 'SetWinLines' (24) is enabled
            * through `options.windowsOptions`.
            */
-          if (this._optionsService.options.windowOptions.setWinLines) {
+          if (this._optionsService.rawOptions.windowOptions.setWinLines) {
             this._bufferService.resize(132, this._bufferService.rows);
             this._onRequestReset.fire();
           }
@@ -2163,7 +2163,7 @@ export class InputHandler extends Disposable implements IInputHandler {
            * This is only active if 'SetWinLines' (24) is enabled
            * through `options.windowsOptions`.
            */
-          if (this._optionsService.options.windowOptions.setWinLines) {
+          if (this._optionsService.rawOptions.windowOptions.setWinLines) {
             this._bufferService.resize(80, this._bufferService.rows);
             this._onRequestReset.fire();
           }
@@ -2740,7 +2740,7 @@ export class InputHandler extends Disposable implements IInputHandler {
    *    Ps >= 24                                                          not implemented
    */
   public windowOptions(params: IParams): boolean {
-    if (!paramToWindowOption(params.params[0], this._optionsService.options.windowOptions)) {
+    if (!paramToWindowOption(params.params[0], this._optionsService.rawOptions.windowOptions)) {
       return true;
     }
     const second = (params.length > 1) ? params.params[1] : 0;

src/common/TestUtils.test.ts

@@ -120,12 +120,13 @@ export class MockLogService implements ILogService {
 
 export class MockOptionsService implements IOptionsService {
   public serviceBrand: any;
-  public options: ITerminalOptions = clone(DEFAULT_OPTIONS);
+  public readonly rawOptions: ITerminalOptions = clone(DEFAULT_OPTIONS);
+  public options: ITerminalOptions = this.rawOptions;
   public onOptionChange: IEvent<string> = new EventEmitter<string>().event;
   constructor(testOptions?: Partial<ITerminalOptions>) {
     if (testOptions) {
       for (const key of Object.keys(testOptions)) {
-        this.options[key] = testOptions[key];
+        this.rawOptions[key] = testOptions[key];
       }
     }
   }

src/common/buffer/Buffer.ts

@@ -107,7 +107,7 @@ export class Buffer implements IBuffer {
       return rows;
     }
 
-    const correctBufferLength = rows + this._optionsService.options.scrollback;
+    const correctBufferLength = rows + this._optionsService.rawOptions.scrollback;
 
     return correctBufferLength > MAX_BUFFER_SIZE ? MAX_BUFFER_SIZE : correctBufferLength;
   }
@@ -172,7 +172,7 @@ export class Buffer implements IBuffer {
       if (this._rows < newRows) {
         for (let y = this._rows; y < newRows; y++) {
           if (this.lines.length < newRows + this.ybase) {
-            if (this._optionsService.options.windowsMode) {
+            if (this._optionsService.rawOptions.windowsMode) {
               // Just add the new missing rows on Windows as conpty reprints the screen with it's
               // view of the world. Once a line enters scrollback for conpty it remains there
               this.lines.push(new BufferLine(newCols, nullCell));
@@ -252,7 +252,7 @@ export class Buffer implements IBuffer {
   }
 
   private get _isReflowEnabled(): boolean {
-    return this._hasScrollback && !this._optionsService.options.windowsMode;
+    return this._hasScrollback && !this._optionsService.rawOptions.windowsMode;
   }
 
   private _reflow(newCols: number, newRows: number): void {
@@ -550,7 +550,7 @@ export class Buffer implements IBuffer {
       i = 0;
     }
 
-    for (; i < this._cols; i += this._optionsService.options.tabStopWidth) {
+    for (; i < this._cols; i += this._optionsService.rawOptions.tabStopWidth) {
       this.tabs[i] = true;
     }
   }

src/common/services/BufferService.ts

@@ -36,8 +36,8 @@ export class BufferService extends Disposable implements IBufferService {
     @IOptionsService private _optionsService: IOptionsService
   ) {
     super();
-    this.cols = Math.max(_optionsService.options.cols || 0, MINIMUM_COLS);
-    this.rows = Math.max(_optionsService.options.rows || 0, MINIMUM_ROWS);
+    this.cols = Math.max(_optionsService.rawOptions.cols || 0, MINIMUM_COLS);
+    this.rows = Math.max(_optionsService.rawOptions.rows || 0, MINIMUM_ROWS);
     this.buffers = new BufferSet(_optionsService, this);
   }
 

src/common/services/CoreService.ts

@@ -62,7 +62,7 @@ export class CoreService extends Disposable implements ICoreService {
 
   public triggerDataEvent(data: string, wasUserInput: boolean = false): void {
     // Prevents all events to pty process if stdin is disabled
-    if (this._optionsService.options.disableStdin) {
+    if (this._optionsService.rawOptions.disableStdin) {
       return;
     }
 
@@ -83,7 +83,7 @@ export class CoreService extends Disposable implements ICoreService {
   }
 
   public triggerBinaryEvent(data: string): void {
-    if (this._optionsService.options.disableStdin) {
+    if (this._optionsService.rawOptions.disableStdin) {
       return;
     }
     this._logService.debug(`sending binary "${data}"`, () => data.split('').map(e => e.charCodeAt(0)));

src/common/services/LogService.ts

@@ -46,7 +46,7 @@ export class LogService implements ILogService {
   }
 
   private _updateLogLevel(): void {
-    this.logLevel = optionsKeyToLogLevel[this._optionsService.options.logLevel];
+    this.logLevel = optionsKeyToLogLevel[this._optionsService.rawOptions.logLevel];
   }
 
   private _evalLazyOptionalParams(optionalParams: any[]): void {

src/common/services/OptionsService.test.ts

@@ -10,7 +10,7 @@ describe('OptionsService', () => {
   describe('constructor', () => {
     const originalError = console.error;
     beforeEach(() => {
-      console.error = () => {};
+      console.error = () => { };
     });
     afterEach(() => {
       console.error = originalError;
@@ -26,7 +26,11 @@ describe('OptionsService', () => {
       assert.equal(optionsService.getOption('cols'), 80);
     });
     it('uses default value if invalid constructor option value passed', () => {
-      assert.equal(new OptionsService({tabStopWidth: 0}).getOption('tabStopWidth'), DEFAULT_OPTIONS.tabStopWidth);
+      assert.equal(new OptionsService({ tabStopWidth: 0 }).getOption('tabStopWidth'), DEFAULT_OPTIONS.tabStopWidth);
+    });
+    it('object.keys return the correct number of options', () => {
+      const optionsService = new OptionsService({ cols: 80, rows: 25 });
+      assert.notEqual(Object.keys(optionsService.options).length, 0);
     });
   });
   describe('setOption', () => {

src/common/services/OptionsService.ts

@@ -60,7 +60,7 @@ const FONT_WEIGHT_OPTIONS: Extract<FontWeight, string>[] = ['normal', 'bold', '1
 export class OptionsService implements IOptionsService {
   public serviceBrand: any;
 
-  private _options: ITerminalOptions;
+  public readonly rawOptions: ITerminalOptions;
   public options: ITerminalOptions;
 
   private _onOptionChange = new EventEmitter<string>();
@@ -68,12 +68,12 @@ export class OptionsService implements IOptionsService {
 
   constructor(options: Partial<ITerminalOptions>) {
     // set the default value of each option
-    this._options = { ...DEFAULT_OPTIONS };
+    const defaultOptions = { ...DEFAULT_OPTIONS };
     for (const key in options) {
-      if (key in this._options) {
+      if (key in defaultOptions) {
         try {
           const newValue = options[key];
-          this._options[key] = this._sanitizeAndValidateOption(key, newValue);
+          defaultOptions[key] = this._sanitizeAndValidateOption(key, newValue);
         } catch (e) {
           console.error(e);
         }
@@ -81,34 +81,39 @@ export class OptionsService implements IOptionsService {
     }
 
     // set up getters and setters for each option
-    this.options = this._setupOptions(this._options);
+    this.rawOptions = defaultOptions;
+    this.options = { ... defaultOptions };
+    this._setupOptions();
   }
 
-  private _setupOptions(options: ITerminalOptions): ITerminalOptions {
-    const copiedOptions = { ... options };
-    for (const propName in copiedOptions) {
-      Object.defineProperty(copiedOptions, propName, {
-        get: () => {
-          if (!(propName in DEFAULT_OPTIONS)) {
-            throw new Error(`No option with key "${propName}"`);
-          }
-          return this._options[propName];
-        },
-        set: (value: any) => {
-          if (!(propName in DEFAULT_OPTIONS)) {
-            throw new Error(`No option with key "${propName}"`);
-          }
+  private _setupOptions(): void {
+    const getter = (propName: string): any => {
+      if (!(propName in DEFAULT_OPTIONS)) {
+        throw new Error(`No option with key "${propName}"`);
+      }
+      return this.rawOptions[propName];
+    };
 
-          value = this._sanitizeAndValidateOption(propName, value);
-          // Don't fire an option change event if they didn't change
-          if (this._options[propName] !== value) {
-            this._options[propName] = value;
-            this._onOptionChange.fire(propName);
-          }
-        }
-      });
+    const setter = (propName: string, value: any): void => {
+      if (!(propName in DEFAULT_OPTIONS)) {
+        throw new Error(`No option with key "${propName}"`);
+      }
+
+      value = this._sanitizeAndValidateOption(propName, value);
+      // Don't fire an option change event if they didn't change
+      if (this.rawOptions[propName] !== value) {
+        this.rawOptions[propName] = value;
+        this._onOptionChange.fire(propName);
+      }
+    };
+
+    for (const propName in this.rawOptions) {
+      const desc = {
+        get: getter.bind(this, propName),
+        set: setter.bind(this, propName)
+      };
+      Object.defineProperty(this.options, propName, desc);
     }
-    return copiedOptions;
   }
 
   public setOption(key: string, value: any): void {

src/common/services/Services.ts

@@ -188,6 +188,12 @@ export const IOptionsService = createDecorator<IOptionsService>('OptionsService'
 export interface IOptionsService {
   serviceBrand: undefined;
 
+  /**
+   * Read only access to the raw options object, this is an internal-only fast path for accessing
+   * single options without any validation as we trust TypeScript to enforce correct usage
+   * internally.
+   */
+  readonly rawOptions: Readonly<ITerminalOptions>;
   readonly options: ITerminalOptions;
 
   readonly onOptionChange: IEvent<string>;