* [pve-devel] [PATCH v2 0/3] make linting threaded
@ 2021-07-19 10:31 Dominik Csapak
2021-07-19 10:31 ` [pve-devel] [PATCH v2 1/3] ship proper nodejs module 'pve-eslint' Dominik Csapak
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Dominik Csapak @ 2021-07-19 10:31 UTC (permalink / raw)
To: pve-devel
NOTE: this series will not build until a 'make buildupstream' is
executed (and 'src/lib/eslint.js' should be committed as well). I
did not send it because it would be too big.
this series convert the package into a proper nodejs module
'pve-eslint', and adds threading to the linting binary
changes from v1:
* convert to nodejs module
* split worker code to own file
* drop hacky self-loading script, and use the 'pve-eslint' module instead
Dominik Csapak (3):
ship proper nodejs module 'pve-eslint'
remove unnecessary eslint.js
use worker_threads for linting
Makefile | 2 +-
debian/control | 7 +-
debian/dirs | 1 +
debian/links | 1 +
debian/rules | 5 +-
patches/0001-adapt-webpack-config.patch | 19 +-
src/Makefile | 15 -
src/{ => bin}/app.js | 40 +-
src/eslint.js | 138871 ---------------------
src/index.js | 5 +
src/lib/worker.js | 27 +
src/package.json | 10 +
12 files changed, 92 insertions(+), 138911 deletions(-)
create mode 100644 debian/dirs
create mode 100644 debian/links
delete mode 100644 src/Makefile
rename src/{ => bin}/app.js (92%)
delete mode 100755 src/eslint.js
create mode 100644 src/index.js
create mode 100644 src/lib/worker.js
create mode 100644 src/package.json
--
2.30.2
^ permalink raw reply [flat|nested] 11+ messages in thread* [pve-devel] [PATCH v2 1/3] ship proper nodejs module 'pve-eslint' 2021-07-19 10:31 [pve-devel] [PATCH v2 0/3] make linting threaded Dominik Csapak @ 2021-07-19 10:31 ` Dominik Csapak 2021-08-25 16:38 ` Thomas Lamprecht 2021-07-19 10:31 ` [pve-devel] [PATCH v2 2/3] remove unnecessary eslint.js Dominik Csapak 2021-07-19 10:31 ` [pve-devel] [PATCH v2 3/3] use worker_threads for linting Dominik Csapak 2 siblings, 1 reply; 11+ messages in thread From: Dominik Csapak @ 2021-07-19 10:31 UTC (permalink / raw) To: pve-devel instead of concatenating the eslint module into our app.js, ship a 'pve-eslint' module that exports the built eslint module to do this, we have to leave the module type on 'umd' instead of changing to 'var' so that nodejs can properly import it. Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> --- Makefile | 2 +- debian/control | 7 +++++-- debian/dirs | 1 + debian/links | 1 + debian/rules | 5 ++++- patches/0001-adapt-webpack-config.patch | 19 +++++-------------- src/Makefile | 15 --------------- src/{ => bin}/app.js | 5 ++++- src/index.js | 3 +++ src/package.json | 9 +++++++++ 10 files changed, 33 insertions(+), 34 deletions(-) create mode 100644 debian/dirs create mode 100644 debian/links delete mode 100644 src/Makefile rename src/{ => bin}/app.js (99%) create mode 100644 src/index.js create mode 100644 src/package.json diff --git a/Makefile b/Makefile index 9dbe3d0..2a6666c 100644 --- a/Makefile +++ b/Makefile @@ -49,7 +49,7 @@ download: # NOTE: needs npm installed, downloads packages from npm .PHONY: buildupstream buildupstream: ${BUILDSRC} - cp ${BUILDSRC}/build/eslint.js ${SRCDIR}/eslint.js + cp ${BUILDSRC}/build/eslint.js ${SRCDIR}/lib/eslint.js ${BUILDSRC}: ${UPSTREAM} patches rm -rf $@ diff --git a/debian/control b/debian/control index 3f9b014..7ea3664 100644 --- a/debian/control +++ b/debian/control @@ -2,13 +2,16 @@ Source: pve-eslint Section: devel Priority: optional Maintainer: Proxmox Support Team <support@proxmox.com> -Build-Depends: debhelper (>= 12~) +Build-Depends: debhelper (>= 12~), + nodejs, + pkg-js-tools (>= 0.8.11) Standards-Version: 4.3.0 Homepage: http://www.proxmox.com Package: pve-eslint Architecture: all -Depends: node-commander, node-colors, nodejs, ${misc:Depends}, +Depends: node-commander, node-colors, nodejs (>= ${nodejs:Version}), ${misc:Depends}, +Provides: ${nodejs:Provides} Description: ESLint for Proxmox Virtual Environment development This package contains a version of eslint used to develop the Proxmox Virtual Environment, and other Proxmox projects, web GUI. diff --git a/debian/dirs b/debian/dirs new file mode 100644 index 0000000..e772481 --- /dev/null +++ b/debian/dirs @@ -0,0 +1 @@ +usr/bin diff --git a/debian/links b/debian/links new file mode 100644 index 0000000..99342ed --- /dev/null +++ b/debian/links @@ -0,0 +1 @@ +usr/share/nodejs/pve-eslint/bin/app.js usr/bin/eslint diff --git a/debian/rules b/debian/rules index 2d33f6a..b4c4090 100755 --- a/debian/rules +++ b/debian/rules @@ -1,4 +1,7 @@ #!/usr/bin/make -f %: - dh $@ + dh $@ --with nodejs + +execute_after_dh_fixperms: + chmod --recursive a+x -- debian/*/usr/share/nodejs/pve-eslint/bin/* diff --git a/patches/0001-adapt-webpack-config.patch b/patches/0001-adapt-webpack-config.patch index 4698e74..b0201e1 100644 --- a/patches/0001-adapt-webpack-config.patch +++ b/patches/0001-adapt-webpack-config.patch @@ -3,21 +3,20 @@ From: Dominik Csapak <d.csapak@proxmox.com> Date: Thu, 2 Apr 2020 07:10:18 +0000 Subject: [PATCH] adapt webpack config -changes to 'var' from 'umd' since we want to use it in the same file adds 'cli-engine' to build (we use it in our wrapper) and target 'node' since we will use it on the cli Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> --- - webpack.config.js | 6 +++--- - 1 file changed, 3 insertions(+), 3 deletions(-) + webpack.config.js | 3 ++- + 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/webpack.config.js b/webpack.config.js -index 29d60cb4..95027075 100644 +index a22c99b..9209159 100644 --- a/webpack.config.js +++ b/webpack.config.js -@@ -2,14 +2,14 @@ - +@@ -4,8 +4,9 @@ const NodePolyfillPlugin = require("node-polyfill-webpack-plugin"); + /** @type {import("webpack").Configuration} */ module.exports = { mode: "none", + target: "node", @@ -27,13 +26,5 @@ index 29d60cb4..95027075 100644 }, output: { filename: "[name].js", - library: "[name]", -- libraryTarget: "umd", -- globalObject: "this" -+ libraryTarget: "var" - }, - module: { - rules: [ -- 2.20.1 - diff --git a/src/Makefile b/src/Makefile deleted file mode 100644 index bef1c57..0000000 diff --git a/src/app.js b/src/bin/app.js similarity index 99% rename from src/app.js rename to src/bin/app.js index 9226234..8a28923 100644 --- a/src/app.js +++ b/src/bin/app.js @@ -1,9 +1,12 @@ -(function() { +#!/usr/bin/env node + +(async function(){ 'use strict'; const path = require('path'); const color = require('colors'); const program = require('commander'); +const eslint = require('pve-eslint'); program .usage('[options] [<file(s) ...>]') diff --git a/src/index.js b/src/index.js new file mode 100644 index 0000000..01b9a1d --- /dev/null +++ b/src/index.js @@ -0,0 +1,3 @@ +const eslint = require('./lib/eslint.js'); + +module.exports = eslint; diff --git a/src/package.json b/src/package.json new file mode 100644 index 0000000..b08184b --- /dev/null +++ b/src/package.json @@ -0,0 +1,9 @@ +{ + "name": "pve-eslint", + "version": "7.28.0", + "files": [ + "index.js", + "bin/app.js", + "lib/eslint.js" + ] +} -- 2.30.2 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [pve-devel] [PATCH v2 1/3] ship proper nodejs module 'pve-eslint' 2021-07-19 10:31 ` [pve-devel] [PATCH v2 1/3] ship proper nodejs module 'pve-eslint' Dominik Csapak @ 2021-08-25 16:38 ` Thomas Lamprecht 2021-08-30 9:17 ` Dominik Csapak 0 siblings, 1 reply; 11+ messages in thread From: Thomas Lamprecht @ 2021-08-25 16:38 UTC (permalink / raw) To: Proxmox VE development discussion, Dominik Csapak On 19/07/2021 12:31, Dominik Csapak wrote: > instead of concatenating the eslint module into our app.js, ship > a 'pve-eslint' module that exports the built eslint module > > to do this, we have to leave the module type on 'umd' instead of > changing to 'var' so that nodejs can properly import it. > > Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> > --- > Makefile | 2 +- Does not applies here, did not really investigated yet though: Applying: ship proper nodejs module 'pve-eslint' Using index info to reconstruct a base tree... error: removal patch leaves file contents error: src/Makefile: patch does not apply > debian/control | 7 +++++-- > debian/dirs | 1 + > debian/links | 1 + > debian/rules | 5 ++++- > patches/0001-adapt-webpack-config.patch | 19 +++++-------------- > src/Makefile | 15 --------------- > src/{ => bin}/app.js | 5 ++++- > src/index.js | 3 +++ > src/package.json | 9 +++++++++ > 10 files changed, 33 insertions(+), 34 deletions(-) > create mode 100644 debian/dirs > create mode 100644 debian/links > delete mode 100644 src/Makefile > rename src/{ => bin}/app.js (99%) > create mode 100644 src/index.js > create mode 100644 src/package.json > > diff --git a/Makefile b/Makefile > index 9dbe3d0..2a6666c 100644 > --- a/Makefile > +++ b/Makefile > @@ -49,7 +49,7 @@ download: > # NOTE: needs npm installed, downloads packages from npm > .PHONY: buildupstream > buildupstream: ${BUILDSRC} > - cp ${BUILDSRC}/build/eslint.js ${SRCDIR}/eslint.js > + cp ${BUILDSRC}/build/eslint.js ${SRCDIR}/lib/eslint.js > > ${BUILDSRC}: ${UPSTREAM} patches > rm -rf $@ > diff --git a/debian/control b/debian/control > index 3f9b014..7ea3664 100644 > --- a/debian/control > +++ b/debian/control > @@ -2,13 +2,16 @@ Source: pve-eslint > Section: devel > Priority: optional > Maintainer: Proxmox Support Team <support@proxmox.com> > -Build-Depends: debhelper (>= 12~) > +Build-Depends: debhelper (>= 12~), > + nodejs, > + pkg-js-tools (>= 0.8.11) > Standards-Version: 4.3.0 > Homepage: http://www.proxmox.com > > Package: pve-eslint > Architecture: all > -Depends: node-commander, node-colors, nodejs, ${misc:Depends}, > +Depends: node-commander, node-colors, nodejs (>= ${nodejs:Version}), ${misc:Depends}, > +Provides: ${nodejs:Provides} > Description: ESLint for Proxmox Virtual Environment development > This package contains a version of eslint used to develop the > Proxmox Virtual Environment, and other Proxmox projects, web GUI. > diff --git a/debian/dirs b/debian/dirs > new file mode 100644 > index 0000000..e772481 > --- /dev/null > +++ b/debian/dirs > @@ -0,0 +1 @@ > +usr/bin > diff --git a/debian/links b/debian/links > new file mode 100644 > index 0000000..99342ed > --- /dev/null > +++ b/debian/links > @@ -0,0 +1 @@ > +usr/share/nodejs/pve-eslint/bin/app.js usr/bin/eslint > diff --git a/debian/rules b/debian/rules > index 2d33f6a..b4c4090 100755 > --- a/debian/rules > +++ b/debian/rules > @@ -1,4 +1,7 @@ > #!/usr/bin/make -f > > %: > - dh $@ > + dh $@ --with nodejs > + > +execute_after_dh_fixperms: > + chmod --recursive a+x -- debian/*/usr/share/nodejs/pve-eslint/bin/* > diff --git a/patches/0001-adapt-webpack-config.patch b/patches/0001-adapt-webpack-config.patch > index 4698e74..b0201e1 100644 > --- a/patches/0001-adapt-webpack-config.patch > +++ b/patches/0001-adapt-webpack-config.patch > @@ -3,21 +3,20 @@ From: Dominik Csapak <d.csapak@proxmox.com> > Date: Thu, 2 Apr 2020 07:10:18 +0000 > Subject: [PATCH] adapt webpack config > > -changes to 'var' from 'umd' since we want to use it in the same file > adds 'cli-engine' to build (we use it in our wrapper) > and target 'node' since we will use it on the cli > > Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> > --- > - webpack.config.js | 6 +++--- > - 1 file changed, 3 insertions(+), 3 deletions(-) > + webpack.config.js | 3 ++- > + 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/webpack.config.js b/webpack.config.js > -index 29d60cb4..95027075 100644 > +index a22c99b..9209159 100644 > --- a/webpack.config.js > +++ b/webpack.config.js > -@@ -2,14 +2,14 @@ > - > +@@ -4,8 +4,9 @@ const NodePolyfillPlugin = require("node-polyfill-webpack-plugin"); > + /** @type {import("webpack").Configuration} */ > module.exports = { > mode: "none", > + target: "node", > @@ -27,13 +26,5 @@ index 29d60cb4..95027075 100644 > }, > output: { > filename: "[name].js", > - library: "[name]", > -- libraryTarget: "umd", > -- globalObject: "this" > -+ libraryTarget: "var" > - }, > - module: { > - rules: [ > -- > 2.20.1 > - > diff --git a/src/Makefile b/src/Makefile > deleted file mode 100644 > index bef1c57..0000000 > diff --git a/src/app.js b/src/bin/app.js > similarity index 99% > rename from src/app.js > rename to src/bin/app.js > index 9226234..8a28923 100644 > --- a/src/app.js > +++ b/src/bin/app.js > @@ -1,9 +1,12 @@ > -(function() { > +#!/usr/bin/env node > + > +(async function(){ > 'use strict'; > > const path = require('path'); > const color = require('colors'); > const program = require('commander'); > +const eslint = require('pve-eslint'); > > program > .usage('[options] [<file(s) ...>]') > diff --git a/src/index.js b/src/index.js > new file mode 100644 > index 0000000..01b9a1d > --- /dev/null > +++ b/src/index.js > @@ -0,0 +1,3 @@ > +const eslint = require('./lib/eslint.js'); > + > +module.exports = eslint; > diff --git a/src/package.json b/src/package.json > new file mode 100644 > index 0000000..b08184b > --- /dev/null > +++ b/src/package.json > @@ -0,0 +1,9 @@ > +{ > + "name": "pve-eslint", > + "version": "7.28.0", > + "files": [ > + "index.js", > + "bin/app.js", > + "lib/eslint.js" > + ] > +} > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [pve-devel] [PATCH v2 1/3] ship proper nodejs module 'pve-eslint' 2021-08-25 16:38 ` Thomas Lamprecht @ 2021-08-30 9:17 ` Dominik Csapak 2021-08-30 9:25 ` Thomas Lamprecht 0 siblings, 1 reply; 11+ messages in thread From: Dominik Csapak @ 2021-08-30 9:17 UTC (permalink / raw) To: Thomas Lamprecht, Proxmox VE development discussion On 8/25/21 18:38, Thomas Lamprecht wrote: > On 19/07/2021 12:31, Dominik Csapak wrote: >> instead of concatenating the eslint module into our app.js, ship >> a 'pve-eslint' module that exports the built eslint module >> >> to do this, we have to leave the module type on 'umd' instead of >> changing to 'var' so that nodejs can properly import it. >> >> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> >> --- >> Makefile | 2 +- > > Does not applies here, did not really investigated yet though: > > Applying: ship proper nodejs module 'pve-eslint' > Using index info to reconstruct a base tree... > error: removal patch leaves file contents > error: src/Makefile: patch does not apply > > ah yes, sorry, i created the patches with '-D' since the second patch would not fit on the mailing list shall i send 1/3 and 3/3 again with out -D ? the only conflict is that src/Makefile is deleted (2/3 can be done separately anyway) ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [pve-devel] [PATCH v2 1/3] ship proper nodejs module 'pve-eslint' 2021-08-30 9:17 ` Dominik Csapak @ 2021-08-30 9:25 ` Thomas Lamprecht 2021-08-30 10:06 ` Dominik Csapak 0 siblings, 1 reply; 11+ messages in thread From: Thomas Lamprecht @ 2021-08-30 9:25 UTC (permalink / raw) To: Dominik Csapak, Proxmox VE development discussion Am 8/30/21 um 11:17 AM schrieb Dominik Csapak: > On 8/25/21 18:38, Thomas Lamprecht wrote: >> On 19/07/2021 12:31, Dominik Csapak wrote: >>> instead of concatenating the eslint module into our app.js, ship >>> a 'pve-eslint' module that exports the built eslint module >>> >>> to do this, we have to leave the module type on 'umd' instead of >>> changing to 'var' so that nodejs can properly import it. >>> >>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> >>> --- >>> Makefile | 2 +- >> >> Does not applies here, did not really investigated yet though: >> >> Applying: ship proper nodejs module 'pve-eslint' >> Using index info to reconstruct a base tree... >> error: removal patch leaves file contents >> error: src/Makefile: patch does not apply >> >> > > ah yes, sorry, i created the patches with '-D' > since the second patch would not fit on the mailing list > > shall i send 1/3 and 3/3 again with out -D ? > the only conflict is that src/Makefile is deleted > (2/3 can be done separately anyway) > > I can also pull this from a staff repo if you prefer that? ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [pve-devel] [PATCH v2 1/3] ship proper nodejs module 'pve-eslint' 2021-08-30 9:25 ` Thomas Lamprecht @ 2021-08-30 10:06 ` Dominik Csapak 2021-08-30 10:36 ` [pve-devel] applied: " Thomas Lamprecht 0 siblings, 1 reply; 11+ messages in thread From: Dominik Csapak @ 2021-08-30 10:06 UTC (permalink / raw) To: Thomas Lamprecht, Proxmox VE development discussion On 8/30/21 11:25, Thomas Lamprecht wrote: > Am 8/30/21 um 11:17 AM schrieb Dominik Csapak: >> On 8/25/21 18:38, Thomas Lamprecht wrote: >>> On 19/07/2021 12:31, Dominik Csapak wrote: >>>> instead of concatenating the eslint module into our app.js, ship >>>> a 'pve-eslint' module that exports the built eslint module >>>> >>>> to do this, we have to leave the module type on 'umd' instead of >>>> changing to 'var' so that nodejs can properly import it. >>>> >>>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> >>>> --- >>>> Makefile | 2 +- >>> >>> Does not applies here, did not really investigated yet though: >>> >>> Applying: ship proper nodejs module 'pve-eslint' >>> Using index info to reconstruct a base tree... >>> error: removal patch leaves file contents >>> error: src/Makefile: patch does not apply >>> >>> >> >> ah yes, sorry, i created the patches with '-D' >> since the second patch would not fit on the mailing list >> >> shall i send 1/3 and 3/3 again with out -D ? >> the only conflict is that src/Makefile is deleted >> (2/3 can be done separately anyway) >> >> > > I can also pull this from a staff repo if you prefer that? > ok, its the 'nodejs' branch of my staff repo for 'pve-eslint' ^ permalink raw reply [flat|nested] 11+ messages in thread
* [pve-devel] applied: Re: [PATCH v2 1/3] ship proper nodejs module 'pve-eslint' 2021-08-30 10:06 ` Dominik Csapak @ 2021-08-30 10:36 ` Thomas Lamprecht 2021-08-30 10:43 ` Dominik Csapak 0 siblings, 1 reply; 11+ messages in thread From: Thomas Lamprecht @ 2021-08-30 10:36 UTC (permalink / raw) To: Proxmox VE development discussion, Dominik Csapak Am 8/30/21 um 12:06 PM schrieb Dominik Csapak: > On 8/30/21 11:25, Thomas Lamprecht wrote: >> Am 8/30/21 um 11:17 AM schrieb Dominik Csapak: >>> On 8/25/21 18:38, Thomas Lamprecht wrote: >>>> On 19/07/2021 12:31, Dominik Csapak wrote: >>>>> instead of concatenating the eslint module into our app.js, ship >>>>> a 'pve-eslint' module that exports the built eslint module >>>>> >>>>> to do this, we have to leave the module type on 'umd' instead of >>>>> changing to 'var' so that nodejs can properly import it. >>>>> >>>>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> >>>>> --- >>>>> Makefile | 2 +- >>>> >>>> Does not applies here, did not really investigated yet though: >>>> >>>> Applying: ship proper nodejs module 'pve-eslint' >>>> Using index info to reconstruct a base tree... >>>> error: removal patch leaves file contents >>>> error: src/Makefile: patch does not apply >>>> >>>> >>> >>> ah yes, sorry, i created the patches with '-D' >>> since the second patch would not fit on the mailing list >>> >>> shall i send 1/3 and 3/3 again with out -D ? >>> the only conflict is that src/Makefile is deleted >>> (2/3 can be done separately anyway) >>> >>> >> >> I can also pull this from a staff repo if you prefer that? >> > > ok, its the 'nodejs' branch of my staff repo for 'pve-eslint' > great, applied, thanks! ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [pve-devel] applied: Re: [PATCH v2 1/3] ship proper nodejs module 'pve-eslint' 2021-08-30 10:36 ` [pve-devel] applied: " Thomas Lamprecht @ 2021-08-30 10:43 ` Dominik Csapak 2021-08-30 11:03 ` Thomas Lamprecht 0 siblings, 1 reply; 11+ messages in thread From: Dominik Csapak @ 2021-08-30 10:43 UTC (permalink / raw) To: Thomas Lamprecht, Proxmox VE development discussion On 8/30/21 12:36, Thomas Lamprecht wrote: > Am 8/30/21 um 12:06 PM schrieb Dominik Csapak: >> On 8/30/21 11:25, Thomas Lamprecht wrote: >>> Am 8/30/21 um 11:17 AM schrieb Dominik Csapak: >>>> On 8/25/21 18:38, Thomas Lamprecht wrote: >>>>> On 19/07/2021 12:31, Dominik Csapak wrote: >>>>>> instead of concatenating the eslint module into our app.js, ship >>>>>> a 'pve-eslint' module that exports the built eslint module >>>>>> >>>>>> to do this, we have to leave the module type on 'umd' instead of >>>>>> changing to 'var' so that nodejs can properly import it. >>>>>> >>>>>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> >>>>>> --- >>>>>> Makefile | 2 +- >>>>> >>>>> Does not applies here, did not really investigated yet though: >>>>> >>>>> Applying: ship proper nodejs module 'pve-eslint' >>>>> Using index info to reconstruct a base tree... >>>>> error: removal patch leaves file contents >>>>> error: src/Makefile: patch does not apply >>>>> >>>>> >>>> >>>> ah yes, sorry, i created the patches with '-D' >>>> since the second patch would not fit on the mailing list >>>> >>>> shall i send 1/3 and 3/3 again with out -D ? >>>> the only conflict is that src/Makefile is deleted >>>> (2/3 can be done separately anyway) >>>> >>>> >>> >>> I can also pull this from a staff repo if you prefer that? >>> >> >> ok, its the 'nodejs' branch of my staff repo for 'pve-eslint' >> > > great, applied, thanks! > thx, please do not forget to run 'make buildupstream' and commiting the eslint file before bumping :) (or i can do it and push to my staff repo if you want) ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [pve-devel] applied: Re: [PATCH v2 1/3] ship proper nodejs module 'pve-eslint' 2021-08-30 10:43 ` Dominik Csapak @ 2021-08-30 11:03 ` Thomas Lamprecht 0 siblings, 0 replies; 11+ messages in thread From: Thomas Lamprecht @ 2021-08-30 11:03 UTC (permalink / raw) To: Dominik Csapak, Proxmox VE development discussion Am 8/30/21 um 12:43 PM schrieb Dominik Csapak: > On 8/30/21 12:36, Thomas Lamprecht wrote: >> Am 8/30/21 um 12:06 PM schrieb Dominik Csapak: >>> On 8/30/21 11:25, Thomas Lamprecht wrote: >>>> Am 8/30/21 um 11:17 AM schrieb Dominik Csapak: >>>>> On 8/25/21 18:38, Thomas Lamprecht wrote: >>>>>> On 19/07/2021 12:31, Dominik Csapak wrote: >>>>>>> instead of concatenating the eslint module into our app.js, ship >>>>>>> a 'pve-eslint' module that exports the built eslint module >>>>>>> >>>>>>> to do this, we have to leave the module type on 'umd' instead of >>>>>>> changing to 'var' so that nodejs can properly import it. >>>>>>> >>>>>>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> >>>>>>> --- >>>>>>> Makefile | 2 +- >>>>>> >>>>>> Does not applies here, did not really investigated yet though: >>>>>> >>>>>> Applying: ship proper nodejs module 'pve-eslint' >>>>>> Using index info to reconstruct a base tree... >>>>>> error: removal patch leaves file contents >>>>>> error: src/Makefile: patch does not apply >>>>>> >>>>>> >>>>> >>>>> ah yes, sorry, i created the patches with '-D' >>>>> since the second patch would not fit on the mailing list >>>>> >>>>> shall i send 1/3 and 3/3 again with out -D ? >>>>> the only conflict is that src/Makefile is deleted >>>>> (2/3 can be done separately anyway) >>>>> >>>>> >>>> >>>> I can also pull this from a staff repo if you prefer that? >>>> >>> >>> ok, its the 'nodejs' branch of my staff repo for 'pve-eslint' >>> >> >> great, applied, thanks! >> > > thx, please do not forget to run 'make buildupstream' and > commiting the eslint file before bumping :) > (or i can do it and push to my staff repo if you want) > Without that it did not build anyway (I wanted to test it), but thanks for the reminder - updated+committed build got pushed out now. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [pve-devel] [PATCH v2 2/3] remove unnecessary eslint.js 2021-07-19 10:31 [pve-devel] [PATCH v2 0/3] make linting threaded Dominik Csapak 2021-07-19 10:31 ` [pve-devel] [PATCH v2 1/3] ship proper nodejs module 'pve-eslint' Dominik Csapak @ 2021-07-19 10:31 ` Dominik Csapak 2021-07-19 10:31 ` [pve-devel] [PATCH v2 3/3] use worker_threads for linting Dominik Csapak 2 siblings, 0 replies; 11+ messages in thread From: Dominik Csapak @ 2021-07-19 10:31 UTC (permalink / raw) To: pve-devel Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> --- src/eslint.js | 138871 ----------------------------------------------- 1 file changed, 138871 deletions(-) delete mode 100755 src/eslint.js diff --git a/src/eslint.js b/src/eslint.js deleted file mode 100755 index 6115dea..0000000 -- 2.30.2 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [pve-devel] [PATCH v2 3/3] use worker_threads for linting 2021-07-19 10:31 [pve-devel] [PATCH v2 0/3] make linting threaded Dominik Csapak 2021-07-19 10:31 ` [pve-devel] [PATCH v2 1/3] ship proper nodejs module 'pve-eslint' Dominik Csapak 2021-07-19 10:31 ` [pve-devel] [PATCH v2 2/3] remove unnecessary eslint.js Dominik Csapak @ 2021-07-19 10:31 ` Dominik Csapak 2 siblings, 0 replies; 11+ messages in thread From: Dominik Csapak @ 2021-07-19 10:31 UTC (permalink / raw) To: pve-devel instead linting all files in the main thread, use worker threads for that (4 by default) and add the '-t' switch to able to control that a basic benchmark of eslint of pve-manager showed some performance gains: Benchmark #1: Current Time (mean ± σ): 6.468 s ± 0.116 s [User: 9.803 s, System: 0.333 s] Range (min … max): 6.264 s … 6.647 s 10 runs Benchmark #2: 2Threads Time (mean ± σ): 4.509 s ± 0.106 s [User: 12.706 s, System: 0.530 s] Range (min … max): 4.335 s … 4.674 s 10 runs Benchmark #3: 4Threads Time (mean ± σ): 3.471 s ± 0.033 s [User: 16.390 s, System: 0.630 s] Range (min … max): 3.431 s … 3.542 s 10 runs Benchmark #4: 8Threads Time (mean ± σ): 2.880 s ± 0.044 s [User: 22.454 s, System: 0.938 s] Range (min … max): 2.813 s … 2.964 s 10 runs Summary '8Threads' ran 1.21 ± 0.02 times faster than '4Threads' 1.57 ± 0.04 times faster than '2Threads' 2.25 ± 0.05 times faster than 'Current' after 8 threads, there were no real performance benefits since the overhead to load the module seems to be the biggest factor. Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> --- src/bin/app.js | 35 +++++++++++++++++++++++++++++------ src/index.js | 2 ++ src/lib/worker.js | 27 +++++++++++++++++++++++++++ src/package.json | 3 ++- 4 files changed, 60 insertions(+), 7 deletions(-) create mode 100644 src/lib/worker.js diff --git a/src/bin/app.js b/src/bin/app.js index 8a28923..10e7e6a 100644 --- a/src/bin/app.js +++ b/src/bin/app.js @@ -6,6 +6,7 @@ const path = require('path'); const color = require('colors'); const program = require('commander'); +const worker = require('worker_threads'); const eslint = require('pve-eslint'); program @@ -14,6 +15,7 @@ program .option('-e, --extend <configfile>', 'uses <configfile> ontop of default eslint config.') .option('-f, --fix', 'if set, fixes will be applied.') .option('-s, --strict', 'if set, also exit uncleanly on warnings') + .option('-t, --threads <threads>', 'how many worker_threads should be used (default=4)') .option('--output-config', 'if set, only output the config as JSON and exit.') ; @@ -42,6 +44,11 @@ if (!paths.length) { paths = [process.cwd()]; } +let threadCount = 4; +if (program.threads) { + threadCount = program.threads; +} + const defaultConfig = { parserOptions: { ecmaVersion: 2020, @@ -283,20 +290,36 @@ if (program.outputConfig) { process.exit(0); } -const cli = new eslint.CLIEngine({ +const cliOptions = { baseConfig: config, useEslintrc: true, fix: !!program.fix, cwd: process.cwd(), -}); +}; + +let promises = []; +let filesPerThread = Math.round(paths.length / threadCount); +for (let i = 0; i < (threadCount - 1); i++) { + let files = paths.splice(0, filesPerThread); + promises.push(eslint.createWorker({ + cliOptions, + files + })); +} + +// the remaining paths +promises.push(eslint.createWorker({ + cliOptions, + files: paths +})); -const report = cli.executeOnFiles(paths); +let results = (await Promise.all(promises)).map(res => res.results).flat(1); let exitcode = 0; let files_err = [], files_warn = [], files_ok = []; let fixes = 0; console.log('------------------------------------------------------------'); -report.results.forEach(function(result) { +results.forEach(function(result) { let filename = path.relative(process.cwd(), result.filePath); let msgs = result.messages; let max_sev = 0; @@ -348,7 +371,7 @@ report.results.forEach(function(result) { console.log('------------------------------------------------------------'); }); -if (report.results.length > 1) { +if (results.length > 1) { console.log(`${color.bold(files_ok.length + files_err.length)} files:`); if (files_err.length > 0) { console.log(color.red(` ${color.bold(files_err.length)} files have Errors`)); @@ -367,7 +390,7 @@ console.log('------------------------------------------------------------'); if (program.fix) { if (fixes > 0) { console.log(`Writing ${color.bold(fixes)} fixed files...`); - eslint.CLIEngine.outputFixes(report); + eslint.CLIEngine.outputFixes({ results }); console.log('Done'); } else { console.log("No fixable Errors/Warnings found."); diff --git a/src/index.js b/src/index.js index 01b9a1d..311ae38 100644 --- a/src/index.js +++ b/src/index.js @@ -1,3 +1,5 @@ const eslint = require('./lib/eslint.js'); +const createWorker = require('./lib/worker.js'); module.exports = eslint; +module.exports.createWorker = createWorker; diff --git a/src/lib/worker.js b/src/lib/worker.js new file mode 100644 index 0000000..9a8c955 --- /dev/null +++ b/src/lib/worker.js @@ -0,0 +1,27 @@ +'use strict'; + +const worker = require('worker_threads'); + +if (!worker.isMainThread) { + const eslint = require('pve-eslint'); + const data = worker.workerData; + const cli = new eslint.CLIEngine(data.cliOptions); + const report = cli.executeOnFiles(data.files); + worker.parentPort.postMessage(report); +} else { + module.exports = async function createWorker(workerData) { + return new Promise((resolve, reject) => { + const child = new worker.Worker(__filename, + { + workerData, + }, + ); + child.on('message', resolve); + child.on('error', reject); + child.on('exit', (code) => { + if (code !== 0) {reject(new Error(`Worker stopped with exit code ${code}`));} + }); + }); + } +} + diff --git a/src/package.json b/src/package.json index b08184b..e912069 100644 --- a/src/package.json +++ b/src/package.json @@ -4,6 +4,7 @@ "files": [ "index.js", "bin/app.js", - "lib/eslint.js" + "lib/eslint.js", + "lib/worker.js" ] } -- 2.30.2 ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-08-30 11:04 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-07-19 10:31 [pve-devel] [PATCH v2 0/3] make linting threaded Dominik Csapak 2021-07-19 10:31 ` [pve-devel] [PATCH v2 1/3] ship proper nodejs module 'pve-eslint' Dominik Csapak 2021-08-25 16:38 ` Thomas Lamprecht 2021-08-30 9:17 ` Dominik Csapak 2021-08-30 9:25 ` Thomas Lamprecht 2021-08-30 10:06 ` Dominik Csapak 2021-08-30 10:36 ` [pve-devel] applied: " Thomas Lamprecht 2021-08-30 10:43 ` Dominik Csapak 2021-08-30 11:03 ` Thomas Lamprecht 2021-07-19 10:31 ` [pve-devel] [PATCH v2 2/3] remove unnecessary eslint.js Dominik Csapak 2021-07-19 10:31 ` [pve-devel] [PATCH v2 3/3] use worker_threads for linting Dominik Csapak
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox