You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@echarts.apache.org by GitBox <gi...@apache.org> on 2020/09/24 07:47:11 UTC
[GitHub] [incubator-echarts] pissang opened a new pull request #13339: [5.0] [Performance] Remove chunk in List storage
pissang opened a new pull request #13339:
URL: https://github.com/apache/incubator-echarts/pull/13339
This pull request is about to remove chunks in List. It makes the storage structure much flatter. And brings less and more maintainable code and faster access to storage.
The following screenshot can demonstrate the performance improvement:
#### Before & After
<img src="https://user-images.githubusercontent.com/841551/94115931-f818dc00-fe7c-11ea-830b-78b138dd1f2e.png"
width= "50%"/><img src="https://user-images.githubusercontent.com/841551/94115724-b6883100-fe7c-11ea-8e2f-bded14d14d23.png"
width= "50%"/>
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [incubator-echarts] 100pah commented on a change in pull request #13339: [5.0] [Performance] Remove chunk in List storage
Posted by GitBox <gi...@apache.org>.
100pah commented on a change in pull request #13339:
URL: https://github.com/apache/incubator-echarts/pull/13339#discussion_r494795805
##########
File path: src/data/List.ts
##########
@@ -611,92 +593,90 @@ class List<
this._idDimIdx = i;
}
- if (!storage[dim]) {
- const store: DataValueChunk[] = [];
- storage[dim] = store;
- storageArr.push(store);
- }
-
- prepareChunks(storage, dimInfo, chunkSize, originalChunkCount, end);
-
- this._chunkCount = storage[dim].length;
+ prepareStorage(storage, dimInfo, end, append);
}
- const rawExtentArr = zrUtil.map(dimensions, (dim) => {
- return rawExtent[dim];
+ const storageArr = this._storageArr = map(dimensions, (dim) => {
+ return storage[dim];
});
- let dataItem = [] as OptionDataItem;
- for (let idx = start; idx < end; idx++) {
- // NOTICE: Try not to write things into dataItem
- dataItem = rawData.getItem(idx, dataItem);
- // Each data item is value
- // [1, 2]
- // 2
- // Bar chart, line chart which uses category axis
- // only gives the 'y' value. 'x' value is the indices of category
- // Use a tempValue to normalize the value to be a (x, y) value
- const chunkIndex = mathFloor(idx / chunkSize);
- const chunkOffset = idx % chunkSize;
-
- // Store the data by dimensions
- for (let dimIdx = 0; dimIdx < dimLen; dimIdx++) {
- const dim = dimensions[dimIdx];
- const dimStorage = storageArr[dimIdx][chunkIndex];
- // PENDING NULL is empty or zero
- const val = this._dimValueGetter(dataItem, dim, idx, dimIdx) as ParsedValueNumeric;
- dimStorage[chunkOffset] = val;
+ const rawExtentArr = map(dimensions, (dim) => {
+ return rawExtent[dim];
+ });
- const dimRawExtent = rawExtentArr[dimIdx];
- val < dimRawExtent[0] && (dimRawExtent[0] = val);
- val > dimRawExtent[1] && (dimRawExtent[1] = val);
- }
+ if (rawData.getStorage) {
Review comment:
It might make readers confused when named as `getStorage`.
Because the storage is created by `List` rather than `rawData` and already has value in it.
Probably it's better to name it as `fillStorage`?
##########
File path: src/data/List.ts
##########
@@ -213,7 +213,7 @@ class List<
private _count: number = 0;
private _rawCount: number = 0;
private _storage: DataStorage = {};
- private _storageArr: DataValueChunk[][] = [];
Review comment:
It's would be great if making comment about:
why using _strorageArr (like "optimize xxx")
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [incubator-echarts] echarts-bot[bot] commented on pull request #13339: [5.0] [Performance] Remove chunk in List storage
Posted by GitBox <gi...@apache.org>.
echarts-bot[bot] commented on pull request #13339:
URL: https://github.com/apache/incubator-echarts/pull/13339#issuecomment-698177175
Thanks for your contribution!
The community will review it ASAP. In the meanwhile, please checkout [the coding standard](https://echarts.apache.org/en/coding-standard.html) and Wiki about [How to make a pull request](https://github.com/apache/incubator-echarts/wiki/How-to-make-a-pull-request).
The pull request is marked to be `PR: author is committer` because you are a committer of this project.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [incubator-echarts] 100pah commented on a change in pull request #13339: [5.0] [Performance] Remove chunk in List storage
Posted by GitBox <gi...@apache.org>.
100pah commented on a change in pull request #13339:
URL: https://github.com/apache/incubator-echarts/pull/13339#discussion_r494795805
##########
File path: src/data/List.ts
##########
@@ -611,92 +593,90 @@ class List<
this._idDimIdx = i;
}
- if (!storage[dim]) {
- const store: DataValueChunk[] = [];
- storage[dim] = store;
- storageArr.push(store);
- }
-
- prepareChunks(storage, dimInfo, chunkSize, originalChunkCount, end);
-
- this._chunkCount = storage[dim].length;
+ prepareStorage(storage, dimInfo, end, append);
}
- const rawExtentArr = zrUtil.map(dimensions, (dim) => {
- return rawExtent[dim];
+ const storageArr = this._storageArr = map(dimensions, (dim) => {
+ return storage[dim];
});
- let dataItem = [] as OptionDataItem;
- for (let idx = start; idx < end; idx++) {
- // NOTICE: Try not to write things into dataItem
- dataItem = rawData.getItem(idx, dataItem);
- // Each data item is value
- // [1, 2]
- // 2
- // Bar chart, line chart which uses category axis
- // only gives the 'y' value. 'x' value is the indices of category
- // Use a tempValue to normalize the value to be a (x, y) value
- const chunkIndex = mathFloor(idx / chunkSize);
- const chunkOffset = idx % chunkSize;
-
- // Store the data by dimensions
- for (let dimIdx = 0; dimIdx < dimLen; dimIdx++) {
- const dim = dimensions[dimIdx];
- const dimStorage = storageArr[dimIdx][chunkIndex];
- // PENDING NULL is empty or zero
- const val = this._dimValueGetter(dataItem, dim, idx, dimIdx) as ParsedValueNumeric;
- dimStorage[chunkOffset] = val;
+ const rawExtentArr = map(dimensions, (dim) => {
+ return rawExtent[dim];
+ });
- const dimRawExtent = rawExtentArr[dimIdx];
- val < dimRawExtent[0] && (dimRawExtent[0] = val);
- val > dimRawExtent[1] && (dimRawExtent[1] = val);
- }
+ if (rawData.getStorage) {
Review comment:
It might make readers confused when named as `getStorage`.
Because the storage is created by `List` rather than `rawData` and already has value in it.
Probably it's better to name it as `fillStorage`?
##########
File path: src/data/List.ts
##########
@@ -213,7 +213,7 @@ class List<
private _count: number = 0;
private _rawCount: number = 0;
private _storage: DataStorage = {};
- private _storageArr: DataValueChunk[][] = [];
Review comment:
It's would be great if making comment about:
why using _strorageArr (like "optimize xxx")
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [incubator-echarts] echarts-bot[bot] commented on pull request #13339: [5.0] [Performance] Remove chunk in List storage
Posted by GitBox <gi...@apache.org>.
echarts-bot[bot] commented on pull request #13339:
URL: https://github.com/apache/incubator-echarts/pull/13339#issuecomment-698876983
Congratulations! Your PR has been merged. Thanks for your contribution! 👍
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [incubator-echarts] pissang merged pull request #13339: [5.0] [Performance] Remove chunk in List storage
Posted by GitBox <gi...@apache.org>.
pissang merged pull request #13339:
URL: https://github.com/apache/incubator-echarts/pull/13339
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [incubator-echarts] pissang merged pull request #13339: [5.0] [Performance] Remove chunk in List storage
Posted by GitBox <gi...@apache.org>.
pissang merged pull request #13339:
URL: https://github.com/apache/incubator-echarts/pull/13339
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org
[GitHub] [incubator-echarts] echarts-bot[bot] commented on pull request #13339: [5.0] [Performance] Remove chunk in List storage
Posted by GitBox <gi...@apache.org>.
echarts-bot[bot] commented on pull request #13339:
URL: https://github.com/apache/incubator-echarts/pull/13339#issuecomment-698177175
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org