Version 3.10.x uses significantly more memory and is significantly slower than 3.9.0

[PatSmuk360]

Our builds recently started failing because we run about 80 Less builds in parallel during our project's build process and the new version of Less.js uses so much memory that Node crashes. We traced the crash to upgrading from Less.js from 3.9.0 to 3.10.3.

I changed our Less script to compile the files sequentially (building 2 files at a time) and sampled Node's memory usage during the process and got the following results:

Less.js seems to use 130% more memory now and takes about 100% longer to compile for us.

Just wondering if you've benchmarked Less.js and whether you see similar results

[matthew-dean]

That's odd. What is your Node version?

[matthew-dean]

@PatSmuk360 Could you test and get a memory profile of 3.10.0 and see if it differs?

[PatSmuk360]

We're running the latest version of 10 (10.16.3).

Heap snapshot before:

Heap snapshot after:

[PatSmuk360]

Also I tried on Node 12.10.0 and it seems a lot worse, hitting 587 MB of memory usage at one point in the sequential build.

[PatSmuk360]

CPU profile before:
CPU-20190916T133934.cpuprofile.zip

CPU profile after:
CPU-20190916T134917.cpuprofile.zip

[matthew-dean]

@PatSmuk360 So, the long and short is that the difference between those versions is that the codebase was transformed to ES6 syntax. Which isn't technically a breaking change, which is why it wasn't a major version.

BUT... my suspicion is that some of the Babel conversions for things like object / array spread syntax are less efficient than the more verbose ES5 versions. That's why I was asking if you could test 3.10.0, because originally I was exporting a transpiled package that was compatible with Node 6 and up, but it broke an integration with a particular library that couldn't handle class syntax. So I went down to Node 4 with no class constructs.

If we can figure out exactly which ES5 transform is not performing well, we could theoretically more granularly set those Babel export settings to a more performant export.

[matthew-dean]

@PatSmuk360 Incidentally, what's that split function that's getting so much time?

[kevinramharak]

@matthew-dean That seems to be the String.prototype.split. If you open the profile in your chrome devtools you can see all the data color coded. I'm trying to alter the profile to link to https://cdn.jsdelivr.net/npm/less@3.10.3/dist/less.cjs.js as its source so it is easier to inspect the bottlenecks. Is there a source map available for the *.cjs.js to *.js file? The https://cdn.jsdelivr.net/npm/less@3.10.3/dist/less.min.js.map file seems to map the .min file to the ES6 source. Maybe we can feed the source map to the dev tools so that we can figure out where transpilation causes bottlenecks.

[kevinramharak]

This line in specific

less.js/lib/less/source-map-output.js

Line 78 in cae5021

sourceLines = inputSource.split('\n');

seems to have an high amount of cpu time. But considering the operation it performs that does not seem out of place to me.

[kevinramharak]

I don't have much experience with heap profiles, but the thing that stands out to me is the increase in the amount of (closures), (system), (array), system / Context. Trying to tie that to the cpu profile it seems that the increase of these objects results in a massive increase in time spent garbage collecting.

[matthew-dean]

@kevinramharak In general, converting an AST like Less's with n depth into a serialized, flatted output tree like CSS involves lots of temporary object creation. So, what could be happening is that with certain transforms, it's adding an additional x amount of objects. Even temporarily, you can get an exponential effect where each node creating even just 2-3x the objects, multiplied by the number of nodes multiplied by the number of times it has to flatten rules... I could see it adding up. We were probably naive in general to essentially think of ES6 syntax as essentially syntactic sugar of ES5 syntax. (JavaScript developers are probably guilty of this in general.) In reality, transpiling newer syntax can create ES5 patterns that are not very performant. For 99% of devs, this isn't a big deal because they're not iterating through that code hundreds or thousands of times per second. But this is my guess as to what's happening, because there were no other major changes.

[matthew-dean]

@kevinramharak Re: the source lines - this is because the original Less parser doesn't track lines / columns of input, so when source mapping was added, it needed to essentially chunk input into lines to figure out how it should map to the original source. This won't be an issue in 4.x+, but that makes sense why it would spend a lot of time there now.

[alecpl]

I'm using less.min.js in a browser and 3.10.3 is two times slower than 2.7.3 I was using before. Both in Chrome and Firefox.

[matthew-dean]

@PatSmuk360 Can you check out this branch? https://github.com/matthew-dean/less.js/tree/3.11.0

In short, Babel's transpilation to ES5 is kinda awful, and uses tons of Object.defineProperty calls to transpile classes. I switched transpilation to TypeScript, which has much saner output of function prototypes.

This is all fine and dandy, but after doing this, Less's browser tests won't run because it uses the very old and outdated PhantomJS, and thus far, no one (including me) has been able to successfully migrate the tests off of PhantomJS onto Headless Chrome.

But, one problem at a time: if the dist source on that branch doesn't have the memory issues, then maybe we could tackle the mess that is browser testing.

[matthew-dean]

I've managed to migrate Less browser tests to Headless Chrome, but in terms of performance / stability, I need a lot of feedback from users before it's safe to merge, because of changing the transpilation pipeline entirely from Babel to TypeScript.

Current branch(es) can be found here: #3442