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

Check performance after styles handling changes #6068

Closed
jodator opened this issue Jan 14, 2020 · 5 comments
Closed

Check performance after styles handling changes #6068

jodator opened this issue Jan 14, 2020 · 5 comments
Assignees
Labels
package:engine type:performance This issue reports a performance issue or a possible performance improvement. type:task This issue reports a chore (non-production change) and other types of "todos".

Comments

@jodator
Copy link
Contributor

jodator commented Jan 14, 2020

Provide a description of the task

@jodator jodator added type:task This issue reports a chore (non-production change) and other types of "todos". package:engine type:performance This issue reports a performance issue or a possible performance improvement. labels Jan 14, 2020
@Reinmar Reinmar added this to the iteration 29 milestone Jan 14, 2020
@Reinmar
Copy link
Member

Reinmar commented Jan 17, 2020

@jodator, @mlewand will work on manual tests for performance-oriented research (#5880) so he can actually verify this case.

@mlewand, I'm sure we have a regression because the new system is quite more powerful than the previous style handling. So, it's not a matter of making sure that we didn't regress, but:

  • estimating how big the regression is,
  • finding some bottlenecks (some obvious improvements points).

Let's see the results and think what we should do.

PS. Please remember that the new style parser is not automatically enabled yet (I think), so ask @jodator how to actually test it.

@jodator
Copy link
Contributor Author

jodator commented Jan 17, 2020

All the notes below are for the state after merging style processing and table properties features:

So the new style parser is enabled by default and does more things in general. However, I've used map instead of events for checking if style processing rules are enabled. So that's one scenario (and looks like the most important one - those paths should be AFAP).

The second scenario is enabling one or more style processing rules (ie by enabling TableProperties, TableCellProperties or Indent plugin) or enabling them manually. So the second path used when fancy colored tables are enabled.

@mlewand
Copy link
Contributor

mlewand commented Jan 21, 2020

This issue is blocked by #5880 as it will add proper manual tests where performance can be evaluated.

@mlewand
Copy link
Contributor

mlewand commented Feb 4, 2020

I have verified while doing #5880 that our changes did not affect performance in a notable way.

@mlewand mlewand closed this as completed Feb 4, 2020
@Reinmar
Copy link
Member

Reinmar commented Feb 13, 2020

I have verified while doing #5880 that our changes did not affect performance in a notable way.

That's a great news. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:engine type:performance This issue reports a performance issue or a possible performance improvement. type:task This issue reports a chore (non-production change) and other types of "todos".
Projects
None yet
Development

No branches or pull requests

3 participants