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

Small speedup on autosizing column width #1178

Merged
merged 3 commits into from
Oct 11, 2023

Conversation

Bykiev
Copy link
Collaborator

@Bykiev Bykiev commented Sep 5, 2023

Small speedup on autosizing column width

Small speedup on autosizing column width
@Bykiev
Copy link
Collaborator Author

Bykiev commented Sep 5, 2023

@tonyqus, we can also improve the speed if we will use parallel programming while looping over the rows. What do you think about it?

@tonyqus
Copy link
Member

tonyqus commented Sep 6, 2023

parallel programming is somewhat dangerous to cause CPU 100% if you don't limit the parallel number.

We can try this. But we may need to carefully test this.

@Bykiev
Copy link
Collaborator Author

Bykiev commented Sep 6, 2023

parallel programming is somewhat dangerous to cause CPU 100% if you don't limit the parallel number.

We can try this. But we may need to carefully test this.

Thanks, I think these experiments for another PR, this one can be merged without any issues

@Bykiev
Copy link
Collaborator Author

Bykiev commented Sep 6, 2023

Thought after applying caching this PR doesn't improve performance, the code is cleaner by removing unused code.

Benchmarks.

Before:

Method RowCount ColumnCount Mean Error StdDev Gen0 Allocated
AutoSizeColumn 1000 5 433.1 ms 6.87 ms 6.75 ms 75000.0000 117.23 MB

After:

Method RowCount ColumnCount Mean Error StdDev Gen0 Allocated
AutoSizeColumn 1000 5 428.3 ms 6.04 ms 5.65 ms 75000.0000 117.23 MB

@tonyqus tonyqus added this to the NPOI 2.7.0 milestone Oct 11, 2023
@tonyqus
Copy link
Member

tonyqus commented Oct 11, 2023

LGTM

@tonyqus tonyqus merged commit 18b5971 into nissl-lab:master Oct 11, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants