-
Notifications
You must be signed in to change notification settings - Fork 93
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
Tested large files for performance #48
Comments
Have you some trouble with large file? |
not necessarily yet, but we need to think about setting up performance tests, once all the major features are complete |
If there is performance problem with big file, I think vscode html language service will have the same problem since XMLDocument is rebuild each time you type some content inside editor (TextDocumentSyncKind.Full). I think to improve performance, TextDocumentSyncKind.Incremental should be used, but I think it's an hard thing to do. |
@angelozerr whatever performance the vscode html has, since the xml parser starts to deviate from it (in many ways), we might see a different behaviour, better or worse. |
Totally agree with you. Thanks for creating this issue. |
After testing largeFile.txt |
15000L is not what I call large :-) I was thinking about 10's of MB (even that's not large, depending on the context) |
@NikolasKomonen thanks for testing that and attached your large file.
Indeed I think it can be a problem, and it was my fear, because we need to manage "incremental" parser which is an hard task I think. BUT I have started to study the problem, and building the XMLDocument directly from the given file takes 1590 ms which is too big. It seems the problem comes from the regexp. I will give you feedback and try to fix the problem. |
@NikolasKomonen I have improved performance, your large file takes 195 ms instead of 1590 ms! Please give me feedback. |
@angelozerr awesome work! This makes the extension much snappier!!! |
Glad it pleases you:) I think now we have the same performance than HTML Language Server.
To be honnest with you, I'm not very familiar with profiler, I have just noticed this hang (at first I though it was because of regexp, but it was about String#substring)
I have tried to check that, but I think it's OK. The main problem is to do a substring from a position to string length (it creates a very large String each time, using Matcher#region locate just the matcher. We could use the same Matcher too, but it seems that it doesn't improve performance. |
@angelozerr This is awesome, great find. |
Thanks guys! |
operations are called at the same time (ex: diagnostic, highlight, documentSymbol created 3 XMLDocument, now one XMLDocument is created). see #48
Here's a site to find xml documents of various sizes, some pretty big: http://aiweb.cs.washington.edu/research/projects/xmltk/xmldata/ Tried with a 30MB doc in vscode. Never reached the point where I got error reported. I'll try with incremental support later |
Cool!
Could you give me the link of your xml that you are testing please. |
Seems the server chokes on the nasa.xml from http://aiweb.cs.washington.edu/research/projects/xmltk/xmldata/www/repository.html#nasa Tried with xmx2GB |
@fbricon I have added the nasa.xml in the test:
|
try validation, formatting, hover... |
Yes sure we need too add thoses tests. But if creation of XMLDocument takes 731ms, I think we will have slow problem. WTP XML Editor cannot open it too your nasa.xml. I fear that it will very hard to support very large file. I think a problem is because it's not incremental. Have you tried with "experimental" incremental support? |
I close this issue since 0.8.0 improves performance and memories and gives the capability to disable outline. |
No description provided.
The text was updated successfully, but these errors were encountered: