Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sortable set #5083

Merged
merged 10 commits into from
Jun 21, 2017
38 changes: 16 additions & 22 deletions lib/Chunk.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,23 +6,29 @@

const util = require("util");
const compareLocations = require("./compareLocations");
const SortableSet = require("./util/SortableSet");
let debugId = 1000;

const byId = (a, b) => {
const sortById = (a, b) => {
if(a.id < b.id) return -1;
if(b.id < a.id) return 1;
return 0;
};

const sortByIdentifier = (a, b) => {
if(a.identifier() > b.identifier()) return 1;
if(a.identifier() < b.identifier()) return -1;
return 0;
};

class Chunk {

constructor(name, module, loc) {
this.id = null;
this.ids = null;
this.debugId = debugId++;
this.name = name;
this._modules = new Set();
this._modulesIsSorted = true;
this._modules = new SortableSet(undefined, sortByIdentifier);
this.entrypoints = [];
this.chunks = [];
this.parents = [];
Expand Down Expand Up @@ -92,7 +98,6 @@ class Chunk {
addModule(module) {
if(!this._modules.has(module)) {
this._modules.add(module);
this._modulesIsSorted = false;
return true;
}
return false;
Expand Down Expand Up @@ -139,8 +144,7 @@ class Chunk {
}

setModules(modules) {
this._modules = new Set(modules);
this._modulesIsSorted = false;
this._modules = new SortableSet(modules, sortByIdentifier);
}

getNumberOfModules() {
Expand All @@ -159,19 +163,9 @@ class Chunk {
return Array.from(this._modules, fn);
}

_ensureModulesSorted() {
if(this._modulesIsSorted) return;
this._modules = new Set(Array.from(this._modules).sort((a, b) => {
if(a.identifier() > b.identifier()) return 1;
if(a.identifier() < b.identifier()) return -1;
return 0;
}));
this._modulesIsSorted = true;
}

compareTo(otherChunk) {
this._ensureModulesSorted();
otherChunk._ensureModulesSorted();
this._modules.sort();
otherChunk._modules.sort();
if(this._modules.size > otherChunk._modules.size) return -1;
if(this._modules.size < otherChunk._modules.size) return 1;
const a = this._modules[Symbol.iterator]();
Expand All @@ -196,7 +190,7 @@ class Chunk {
}

getModulesIdent() {
this._ensureModulesSorted();
this._modules.sort();
let str = "";
this._modules.forEach(m => {
str += m.identifier() + "#";
Expand Down Expand Up @@ -428,7 +422,7 @@ class Chunk {
}

sortItems() {
this._modules = new Set(Array.from(this._modules).sort(byId));
this._modules.sortWith(sortById);
this.origins.sort((a, b) => {
const aIdent = a.module.identifier();
const bIdent = b.module.identifier();
Expand All @@ -440,8 +434,8 @@ class Chunk {
if(origin.reasons)
origin.reasons.sort();
});
this.parents.sort(byId);
this.chunks.sort(byId);
this.parents.sort(sortById);
this.chunks.sort(sortById);
}

toString() {
Expand Down
50 changes: 19 additions & 31 deletions lib/Module.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,24 @@
"use strict";

const util = require("util");

const DependenciesBlock = require("./DependenciesBlock");
const ModuleReason = require("./ModuleReason");
const SortableSet = require("./util/SortableSet");
const Template = require("./Template");

function byId(a, b) {
let debugId = 1000;

const sortById = (a, b) => {
return a.id - b.id;
}
};

function byDebugId(a, b) {
const sortByDebugId = (a, b) => {
return a.debugId - b.debugId;
}

let debugId = 1000;
};

class Module extends DependenciesBlock {

constructor() {
super();
this.context = null;
Expand All @@ -34,9 +37,7 @@ class Module extends DependenciesBlock {
this.used = null;
this.usedExports = null;
this.providedExports = null;
this._chunks = new Set();
this._chunksIsSorted = true;
this._chunksIsSortedByDebugId = true;
this._chunks = new SortableSet(undefined, sortById);
this._chunksDebugIdent = undefined;
this.warnings = [];
this.dependenciesWarnings = [];
Expand All @@ -59,7 +60,6 @@ class Module extends DependenciesBlock {
this.providedExports = null;
this._chunks.clear();
this._chunksDebugIdent = undefined;
this._chunksIsSorted = this._chunksIsSortedByDebugId = false;
super.disconnect();
}

Expand All @@ -71,14 +71,16 @@ class Module extends DependenciesBlock {
this.depth = null;
this._chunks.clear();
this._chunksDebugIdent = undefined;
this._chunksIsSorted = this._chunksIsSortedByDebugId = false;
super.unseal();
}

setChunks(chunks) {
this._chunks = new SortableSet(chunks, sortById);
}

addChunk(chunk) {
this._chunks.add(chunk);
this._chunksDebugIdent = undefined;
this._chunksIsSorted = this._chunksIsSortedByDebugId = false;
}

removeChunk(chunk) {
Expand All @@ -96,7 +98,7 @@ class Module extends DependenciesBlock {

getChunkIdsIdent() {
if(this._chunksDebugIdent !== undefined) return this._chunksDebugIdent;
this._ensureChunksSortedByDebugId();
this._chunks.sortWith(sortByDebugId);
const chunks = this._chunks;
const list = [];
for(const chunk of chunks) {
Expand Down Expand Up @@ -130,8 +132,8 @@ class Module extends DependenciesBlock {

hasEqualsChunks(otherModule) {
if(this._chunks.size !== otherModule._chunks.size) return false;
this._ensureChunksSortedByDebugId();
otherModule._ensureChunksSortedByDebugId();
this._chunks.sortWith(sortByDebugId);
otherModule._chunks.sortWith(sortByDebugId);
const a = this._chunks[Symbol.iterator]();
const b = otherModule._chunks[Symbol.iterator]();
while(true) { // eslint-disable-line
Expand All @@ -142,20 +144,6 @@ class Module extends DependenciesBlock {
}
}

_ensureChunksSorted() {
if(this._chunksIsSorted) return;
this._chunks = new Set(Array.from(this._chunks).sort(byId));
this._chunksIsSortedByDebugId = false;
this._chunksIsSorted = true;
}

_ensureChunksSortedByDebugId() {
if(this._chunksIsSortedByDebugId) return;
this._chunks = new Set(Array.from(this._chunks).sort(byDebugId));
this._chunksIsSorted = false;
this._chunksIsSortedByDebugId = true;
}

addReason(module, dependency) {
this.reasons.push(new ModuleReason(module, dependency));
}
Expand Down Expand Up @@ -221,8 +209,8 @@ class Module extends DependenciesBlock {
sortItems(sortChunks) {
super.sortItems();
if(sortChunks)
this._ensureChunksSorted();
this.reasons.sort((a, b) => byId(a.module, b.module));
this._chunks.sort();
this.reasons.sort((a, b) => sortById(a.module, b.module));
if(Array.isArray(this.usedExports)) {
this.usedExports.sort();
}
Expand Down
2 changes: 1 addition & 1 deletion lib/optimize/ConcatenatedModule.js
Original file line number Diff line number Diff line change
Expand Up @@ -129,13 +129,13 @@ function getPathInAst(ast, node) {
class ConcatenatedModule extends Module {
constructor(rootModule, modules) {
super();
super.setChunks(rootModule._chunks);
this.rootModule = rootModule;
this.modules = modules;
this.usedExports = rootModule.usedExports;
this.providedExports = rootModule.providedExports;
this.optimizationBailout = rootModule.optimizationBailout;
this.used = rootModule.used;
this._chunks = new Set(rootModule._chunks);
this.index = rootModule.index;
this.index2 = rootModule.index2;
this.depth = rootModule.depth;
Expand Down
45 changes: 45 additions & 0 deletions lib/util/SortableSet.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
"use strict";

module.exports = class SortableSet extends Set {

constructor(initialIterable, defaultSort) {
super(initialIterable);
this._sortFn = defaultSort;
this._lastActiveSortFn = null;
}

/**
* @param {any} value - value to add to set
* @returns {SortableSet} - returns itself
*/
add(value) {
this._lastActiveSortFn = null;
super.add(value);
return this;
}

/**
* @param {Function} sortFn - function to sort the set
* @returns {void}
*/
sortWith(sortFn) {
if(this.size === 0 || sortFn === this._lastActiveSortFn) {
// already sorted - nothing to do
return;
}

const sortedArray = Array.from(this).sort(sortFn);
super.clear();
for(let i = 0; i < sortedArray.length; i += 1) {
this.add(sortedArray[i]);
}
this._lastActiveSortFn = sortFn;
}

/**
* @returns {void}
*/
sort() {
this.sortWith(this._sortFn);
}
};
29 changes: 29 additions & 0 deletions test/SortableSet.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/* globals describe, it */
"use strict";

const SortableSet = require("../lib/util/SortableSet");

describe("util/SortableSet", () => {
it("Can be constructed like a normal Set", () => {
const sortableSet = new SortableSet([1, 1, 1, 1, 1, 4, 5, 2], () => {});
Array.from(sortableSet).should.eql([1, 4, 5, 2]);
});

it("Can sort its content", () => {
const sortableSet = new SortableSet(
[1, 1, 1, 6, 6, 1, 1, 4, 5, 2, 3, 8, 5, 7, 9, 0, 3, 1],
(a, b) => a - b
);
sortableSet.sort();
Array.from(sortableSet).should.eql([0, 1, 2, 3, 4, 5, 6, 7, 8, 9]);
});

it("Can sort by a specified function", () => {
const sortableSet = new SortableSet(
[1, 1, 1, 6, 6, 1, 1, 4, 5, 2, 3, 8, 5, 7, 9, 0, 3, 1],
(a, b) => a - b
);
sortableSet.sortWith((a, b) => b - a);
Array.from(sortableSet).should.eql([9, 8, 7, 6, 5, 4, 3, 2, 1, 0]);
});
});
2 changes: 1 addition & 1 deletion test/statsCases/scope-hoisting-multi/expected.txt
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ Child
[0] (webpack)/test/statsCases/scope-hoisting-multi/common_lazy_shared.js 25 bytes {0} {1} {2} [built]
[1] (webpack)/test/statsCases/scope-hoisting-multi/vendor.js 25 bytes {5} [built]
ModuleConcatenation (inner): module is an entrypoint
[2] (webpack)/test/statsCases/scope-hoisting-multi/common.js + 1 modules 62 bytes {4} {3} [built]
[2] (webpack)/test/statsCases/scope-hoisting-multi/common.js + 1 modules 62 bytes {3} {4} [built]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should not happen?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see: bd8c6cf

fix tests that was falsy - per default chunks are flagged as sorted in module
however this only holds true as they are initialized empty. Concatenated module however
has initial modules and therefore is not guaranteed to be ordered, the flags should therfor be false.
Using SortedSet fixes this as a sideeffect

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah okey. 👍

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seem to be different again...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ha yea, it tries to use the sort method from Module but its no longer exposed...

meh, i'd love to just have a compareBy method on them :P maybe in another PR if thats ok :D

[3] (webpack)/test/statsCases/scope-hoisting-multi/common_lazy.js 25 bytes {1} {2} [built]
[4] (webpack)/test/statsCases/scope-hoisting-multi/first.js + 1 modules 238 bytes {4} [built]
ModuleConcatenation (inner): module is an entrypoint
Expand Down