changeset 267:8fea191dbff0

Refactor and clean up jvm-memory Reviewed-by: jkang Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2017-October/025371.html
author Andrew Azores <aazores@redhat.com>
date Wed, 11 Oct 2017 14:59:09 -0400
parents 36e99edc6d66
children 597d7d7e0aff
files mock-api/endpoints/jvm-memory.endpoint.js src/app/components/jvm-info/jvm-memory/en.locale.yaml src/app/components/jvm-info/jvm-memory/jvm-memory.controller.js src/app/components/jvm-info/jvm-memory/jvm-memory.controller.spec.js src/app/components/jvm-info/jvm-memory/jvm-memory.html
diffstat 5 files changed, 152 insertions(+), 60 deletions(-) [+]
line wrap: on
line diff
--- a/mock-api/endpoints/jvm-memory.endpoint.js	Wed Oct 11 14:59:09 2017 -0400
+++ b/mock-api/endpoints/jvm-memory.endpoint.js	Wed Oct 11 14:59:09 2017 -0400
@@ -23,7 +23,7 @@
                 capacity: { $numberLong: (100 * 1024 * 1024).toString() },
                 collector: 'Shenandoah',
                 maxCapacity: { $numberLong: (200 * 1024 * 1024).toString() },
-                name: 'Generation 0',
+                name: 'old',
                 spaces: [
                   {
                     capacity: { $numberLong: (50 * 1024 * 1024).toString() },
@@ -45,7 +45,7 @@
                 capacity: { $numberLong: (200 * 1024 * 1024).toString() },
                 collector: 'Shenandoah',
                 maxCapacity: { $numberLong: (400 * 1024 * 1024).toString() },
-                name: 'Generation 1',
+                name: 'new',
                 spaces: [
                   {
                     capacity: { $numberLong: (200 * 1024 * 1024).toString() },
@@ -60,7 +60,7 @@
                 capacity: { $numberLong: (400 * 1024 * 1024).toString() },
                 collector: 'G1',
                 maxCapacity: { $numberLong: (1600 * 1024 * 1024).toString() },
-                name: 'Generation 2',
+                name: 'newest',
                 spaces: [
                   {
                     capacity: { $numberLong: (50 * 1024 * 1024).toString() },
--- a/src/app/components/jvm-info/jvm-memory/en.locale.yaml	Wed Oct 11 14:59:09 2017 -0400
+++ b/src/app/components/jvm-info/jvm-memory/en.locale.yaml	Wed Oct 11 14:59:09 2017 -0400
@@ -1,7 +1,7 @@
 jvmMemory:
   REFRESH_RATE_LABEL: Refresh Rate
   METASPACE: Metaspace
-  COLLECTOR: '{{name}} ({{collector}})'
+  COLLECTOR: '{{name}} generation ({{collector}} collector)'
   SPACE: Space {{index}}
   X_AXIS_LABEL: Time
   Y_AXIS_LABEL: Bytes
--- a/src/app/components/jvm-info/jvm-memory/jvm-memory.controller.js	Wed Oct 11 14:59:09 2017 -0400
+++ b/src/app/components/jvm-info/jvm-memory/jvm-memory.controller.js	Wed Oct 11 14:59:09 2017 -0400
@@ -26,6 +26,7 @@
  */
 
 import 'c3';
+import _ from 'lodash';
 import services from 'shared/services/services.module.js';
 import filters from 'shared/filters/filters.module.js';
 import service from './jvm-memory.service.js';
@@ -55,6 +56,11 @@
       used: 0,
       total: 0
     };
+    this._metaspaceData = {
+      timestamps: [],
+      used: [],
+      capacity: []
+    };
     this.metaspaceSeriesData = {
       xData: ['timestamp'],
       yData: ['memory']
@@ -97,6 +103,7 @@
       };
     });
     this.spaceBarConfigs = [];
+    this._generationData = new Map();
     this.generationSnapshotData = {};
     this.generationSeriesData = {};
     this.generationLineConfigs = new Map();
@@ -196,49 +203,76 @@
 
   _update () {
     this._jvmMemoryService.getJvmMemory(this.jvmId).then(resp => {
+      let keys = [];
       resp.forEach(update => {
         for (let i = 0; i < update.generations.length; i++) {
           update.generations[i].index = i;
         }
+
+        let timestamp = this._metricToNumber(update.timeStamp);
+
+        let metaspaceUsed = this._metricToNumber(update.metaspaceUsed);
+        let metaspaceCapacity = this._metricToNumber(update.metaspaceCapacity);
+
+        this._metaspaceData.timestamps.push(timestamp);
+        this._metaspaceData.used.push(metaspaceUsed);
+        this._metaspaceData.capacity.push(metaspaceCapacity);
+
+        update.generations.forEach(generation => {
+          generation.spaces.forEach(space => {
+            let key = this._getKey(generation.index, space.index);
+            keys.push(key);
+            if (!this._generationData.has(key)) {
+              this._generationData.set(key, {
+                timestamps: [],
+                used: [],
+                capacity: []
+              });
+            }
+
+            let used = this._metricToNumber(space.used);
+            let capacity = this._metricToNumber(space.capacity);
+            let data = this._generationData.get(key);
+            data.timestamps.push(timestamp);
+            data.used.push(used);
+            data.capacity.push(capacity);
+          });
+        });
       });
-      this._updateLineCharts(resp);
+      this._updateLineCharts(keys);
       this._updateBarCharts(resp[0]);
     });
   }
 
-  _updateLineCharts (updates) {
-    updates.forEach(update => {
-      let timestamp = this._metricToNumber(update.timeStamp);
-      let metaspaceUsed = this._metricToNumber(update.metaspaceUsed);
-
-      this.metaspaceSeriesData.xData.push(timestamp);
-      this.metaspaceSeriesData.yData.push(metaspaceUsed);
+  _getKey (genIndex, spaceIndex) {
+    return `${genIndex}:${spaceIndex}`;
+  }
 
-      update.generations.forEach(generation => {
-        generation.spaces.forEach(space => {
-          let key = `gen-${generation.index}-space-${space.index}`;
-          if (!this.generationSeriesData.hasOwnProperty(key)) {
-            this.generationSeriesData[key] = {
-              xData: ['timestamp'],
-              yData: ['memory']
-            };
-          }
-          this.generationSeriesData[key].xData.push(timestamp);
-          this.generationSeriesData[key].yData.push(this.convertMemStat(space.used));
-        });
-      });
+  _updateLineCharts (keys) {
+    this.metaspaceSeriesData = {
+      xData: ['timestamp'].concat(this._metaspaceData.timestamps),
+      yData: ['memory'].concat(this._metaspaceData.used)
+    };
+
+    keys.forEach(key => {
+      this.generationSeriesData[key] = {
+        xData: ['timestamp'].concat(this._generationData.get(key).timestamps),
+        yData: ['memory'].concat(this._generationData.get(key).used)
+      };
     });
   }
 
   _updateBarCharts (data) {
-    let metaspaceScale = this._scaleBytes.format(data.metaspaceUsed);
-    this.metaspaceSnapshotData.used = this.convertMemStat(data.metaspaceUsed, metaspaceScale.scale);
-    this.metaspaceSnapshotData.total = this.convertMemStat(data.metaspaceCapacity, metaspaceScale.scale);
+    let metaspaceScale = this._scaleBytes.format(_.last(this._metaspaceData.used));
+    this.metaspaceSnapshotData = {
+      used: this.convertMemStat(_.last(this._metaspaceData.used), metaspaceScale.scale),
+      total: this.convertMemStat(_.last(this._metaspaceData.capacity), metaspaceScale.scale)
+    };
 
     // re-assign chart config so that Angular picks up the change. If we only re-assign property values
     // on the same config object, those config updates are not detected and do not reflect in the charts.
     this.metaspaceBarConfig = {
-      chartId: this.metaspaceBarConfig.chartId,
+      chartId: 'metaspaceBarChart',
       units: metaspaceScale.unit
     };
 
@@ -249,7 +283,7 @@
         gen = this.generationSnapshotData[i];
       } else {
         gen = {
-          index: i,
+          index: generation.index,
           name: generation.name,
           collector: generation.collector,
           spaces: []
@@ -257,22 +291,28 @@
       }
       for (let j = 0; j < generation.spaces.length; j++) {
         let space = generation.spaces[j];
-        this._translate('jvmMemory.SPACE', { index: space.index }).then(chartTitle => {
-          let genScale = this._scaleBytes.format(space.used);
+        let key = this._getKey(gen.index, space.index);
+
+        let generationData = this._generationData.get(key);
+        let latestTimestamp = _.last(generationData.timestamp);
+        let latestUsed = _.last(generationData.used);
+        let latestCapacity = _.last(generationData.capacity);
 
-          if (gen.spaces.hasOwnProperty(space.index)) {
-            gen.spaces[space.index].used = this.convertMemStat(space.used, genScale.scale);
-            gen.spaces[space.index].total = this.convertMemStat(space.capacity, genScale.scale);
-          } else {
-            gen.spaces[space.index] = {
-              index: space.index,
-              used: this.convertMemStat(space.used, genScale.scale),
-              total: this.convertMemStat(space.capacity, genScale.scale)
-            };
-          }
+        let genScale = this._scaleBytes.format(latestUsed);
+        if (gen.spaces.hasOwnProperty(space.index)) {
+          gen.spaces[space.index].used = this.convertMemStat(latestUsed, genScale.scale);
+          gen.spaces[space.index].total = this.convertMemStat(latestCapacity, genScale.scale);
+        } else {
+          gen.spaces[space.index] = {
+            index: space.index,
+            used: this.convertMemStat(latestUsed, genScale.scale),
+            total: this.convertMemStat(latestCapacity, genScale.scale)
+          };
+        }
 
-          let spaceKey = 'gen-' + gen.index + '-space-' + space.index;
-          this.spaceBarConfigs[spaceKey] = {
+        this._translate('jvmMemory.SPACE', { index: space.index }).then(chartTitle => {
+          this.spaceBarConfigs[key] = {
+            key: key,
             units: genScale.unit,
             title: chartTitle
           };
--- a/src/app/components/jvm-info/jvm-memory/jvm-memory.controller.spec.js	Wed Oct 11 14:59:09 2017 -0400
+++ b/src/app/components/jvm-info/jvm-memory/jvm-memory.controller.spec.js	Wed Oct 11 14:59:09 2017 -0400
@@ -31,7 +31,7 @@
 
 describe('JvmMemory controller', () => {
 
-  let interval, memSvc, scaleSvc, promise, ctrl, sanitizeSvc, translate;
+  let interval, memSvc, scaleSvc, promise, ctrl, sanitizeSvc, translate, dateFilter, dateFormat;
   beforeEach(() => {
     angular.mock.module(filtersModule);
     angular.mock.module(servicesModule);
@@ -61,16 +61,21 @@
       };
 
       translate = sinon.stub().returns({
-        then: sinon.stub().yields('chart-title')
+        then: sinon.stub().yields('translate-mock')
       });
 
+      dateFilter = sinon.stub().returns('mock-date-filter');
+      dateFormat = { time: { medium: 'date-format-medium' } };
+
       ctrl = $controller('JvmMemoryController', {
         $stateParams: { jvmId: 'foo-jvmId' },
         $interval: interval,
         jvmMemoryService: memSvc,
         scaleBytesService: scaleSvc,
         sanitizeService: sanitizeSvc,
-        $translate: translate
+        $translate: translate,
+        dateFilter: dateFilter,
+        DATE_FORMAT: dateFormat
       });
       ctrl.$onInit();
 
@@ -103,6 +108,51 @@
     });
   });
 
+  describe('metaspaceLineConfig', () => {
+    it('should be assigned after init', () => {
+      ctrl.should.have.ownProperty('metaspaceLineConfig');
+    });
+
+    it('should provide an x-axis tick formatter', () => {
+      let fn = ctrl.metaspaceLineConfig.axis.x.tick.format;
+      dateFilter.should.not.be.called();
+      fn(1234);
+      dateFilter.should.be.calledOnce();
+      dateFilter.should.be.calledWith(1234, dateFormat.time.medium);
+    });
+  });
+
+  describe('getLineConfig', () => {
+    it('should return a chart configuration', () => {
+      let genIndex = 2;
+      let spaceIndex = 4;
+      let cfg = ctrl.getLineConfig(genIndex, spaceIndex);
+      cfg.chartId.should.equal('line-gen-2-space-4');
+    });
+
+    it('should provide an x-axis tick formatter', () => {
+      let genIndex = 2;
+      let spaceIndex = 4;
+      let cfg = ctrl.getLineConfig(genIndex, spaceIndex);
+      let fn = cfg.axis.x.tick.format;
+      dateFilter.should.not.be.called();
+      fn(1234);
+      dateFilter.should.be.calledOnce();
+      dateFilter.should.be.calledWith(1234, dateFormat.time.medium);
+    });
+
+    it('should cache configs', () => {
+      let spy = sinon.spy(ctrl.generationLineConfigs, 'set');
+      let genIndex = 2;
+      let spaceIndex = 4;
+      ctrl.getLineConfig(genIndex, spaceIndex);
+      spy.should.be.calledOnce();
+      ctrl.getLineConfig(genIndex, spaceIndex);
+      spy.should.be.calledOnce();
+      spy.restore();
+    });
+  });
+
   it('should update on init', () => {
     memSvc.getJvmMemory.should.be.calledWith('foo-jvmId');
   });
@@ -190,7 +240,7 @@
         total: 40
       });
       scaleSvc.format.should.be.calledTwice();
-      scaleSvc.format.should.be.calledWithMatch({ $numberLong: (20 * 1024 * 1024).toString() });
+      scaleSvc.format.firstCall.should.be.calledWithMatch(20 * 1024 * 1024);
     });
 
     it('should add generationSnapshotData', () => {
@@ -209,7 +259,7 @@
         }
       });
       scaleSvc.format.should.be.calledTwice();
-      scaleSvc.format.should.be.calledWithMatch({ $numberLong: (30 * 1024 * 1024).toString() });
+      scaleSvc.format.secondCall.should.be.calledWithMatch(30 * 1024 * 1024);
     });
 
     it('should update generationSnapshotData on repeated calls', () => {
@@ -233,10 +283,10 @@
         }
       });
       scaleSvc.format.callCount.should.equal(4);
-      scaleSvc.format.args[0][0].should.deepEqual({ $numberLong: (20 * 1024 * 1024).toString() });
-      scaleSvc.format.args[1][0].should.deepEqual({ $numberLong: (30 * 1024 * 1024).toString() });
-      scaleSvc.format.args[2][0].should.deepEqual({ $numberLong: (20 * 1024 * 1024).toString() });
-      scaleSvc.format.args[3][0].should.deepEqual({ $numberLong: (50 * 1024 * 1024).toString() });
+      scaleSvc.format.args[0][0].should.deepEqual(20 * 1024 * 1024);
+      scaleSvc.format.args[1][0].should.deepEqual(30 * 1024 * 1024);
+      scaleSvc.format.args[2][0].should.deepEqual(20 * 1024 * 1024);
+      scaleSvc.format.args[3][0].should.deepEqual(50 * 1024 * 1024);
     });
   });
 
--- a/src/app/components/jvm-info/jvm-memory/jvm-memory.html	Wed Oct 11 14:59:09 2017 -0400
+++ b/src/app/components/jvm-info/jvm-memory/jvm-memory.html	Wed Oct 11 14:59:09 2017 -0400
@@ -30,9 +30,10 @@
                chart-data="$ctrl.metaspaceSeriesData"
                ></pf-line-chart>
             <pf-utilization-bar-chart
-              layout="{type: 'inline'}"
-              units="$ctrl.metaspaceBarConfig.units"
-              chart-data="$ctrl.metaspaceSnapshotData"></pf-utilization-bar-chart>
+               id="metaspaceBarChart"
+               layout="{type: 'inline'}"
+               units="$ctrl.metaspaceBarConfig.units"
+               chart-data="$ctrl.metaspaceSnapshotData"></pf-utilization-bar-chart>
           </div>
         </div>
       </div>
@@ -58,14 +59,15 @@
                  <pf-line-chart
                     id="line-gen-{{generation.index}}-space-{{space.index}}"
                     config="$ctrl.getLineConfig(generation.index, space.index)"
-                    chart-data="$ctrl.generationSeriesData['gen-' + generation.index + '-space-' + space.index]"
+                    chart-data="$ctrl.generationSeriesData[generation.index + ':' + space.index]"
                    ></pf-line-chart>
                  <pf-utilization-bar-chart
-                    id="bar-gen-{{generation.index}}-space-{{space.index}}"
-                    chart-title="$ctrl.spaceBarConfigs['gen-' + generation.index + '-space-' + space.index].title"
+                    id="{{generation.index}}:{{space.index}}"
+                    chart-title="$ctrl.spaceBarConfigs[generation.index + ':' + space.index].title"
                     layout="{type: 'inline'}"
-                    units="$ctrl.spaceBarConfigs['gen-' + generation.index + '-space-' + space.index].units"
+                    units="$ctrl.spaceBarConfigs[generation.index + ':' + space.index].units"
                     chart-data="space"></pf-utilization-bar-chart>
+                  <hr ng-if="!$last" />
               </div>
             </div>
           </div>