Hi there!
It's about ColladaLoader (I didn't check others).
If you replace the code text.trim().split( /\s+/ ) by text.replace(/\n/g, " ").replace(/ \s+/g, " ").trim().split(" ") (see parseFloat and parseInt functions) the parser will work much faster.
The pattern .split( /\s+/ ) is used in ColladaLoader, MTLLoader, NRRDLoader, OBJLoader, PLYLoader and VTKLoader.
the parser will work much faster.
Can you proof with a benchmark that this statement is actually true? Using jsperf can be useful in this context 👍 . Check out how @leodutra created such benchmarks for his PR #16194.
well, take a look - https://jsperf.com/loader-speed-test/1
Indeed, your version is much faster. I have one question about the code. Why is the first call of replace() necessary? I though the metacharacter \s also contains new line characters. Isn't this code sufficient?
text.replace(/ \s+/g, " ").trim().split(" ")
Indeed, if we need a very reliable version we have to replace all of whitespace characters except space (for example 3ds max exporters use \n pretty often). Then we have to replace two or more spaces. After that we can use split(“ “).
https://jsperf.com/loader-speed-test
text.replace(/\n/g, " ").replace(/\r/g, " ").replace(/\t/g, " ").replace(/\f/g, " ").replace(/ +/g, " ").trim().split(" ");
It seems weird but any other variants \n|\r|\t|\f or [\n\r\t\f] work slower
How about making a new method in LoaderUtils in order to hide this code a bit^^? Then it's also possible to write a unit test. In this way, the loaders can easier share a common implementation.
Sure, why not?
There you go - https://github.com/bbmv/parseNumbers
I think it's better to make a PR with your code. 😇
Do you mean Pull Request? Do I have an access to your repository?
Yes, but special access rights are not necessary. Just fork the repo, make the changes in a feature branch and then create a PR targeting the official dev branch. More information:
https://github.com/mrdoob/three.js/wiki/How-to-contribute-to-three.js
Ahh. I see. Thanks
done
Most helpful comment
well, take a look - https://jsperf.com/loader-speed-test/1