ImportExcel icon indicating copy to clipboard operation
ImportExcel copied to clipboard

Refactor Question - Improving ImportExcel module import time

Open joshooaj opened this issue 2 years ago • 7 comments
trafficstars

Hi @dfinke,

Are you open to a PR to change the way the module is "built" for testing and publishing to reduce the amount of time it takes to import the module?

On my Windows 11 laptop it takes ~7.5 seconds to import the module in PowerShell 5.1 as published to PSGallery. The bulk of that time is spent on dot-sourcing the .PS1 files in the Private, Public, Charting, InferData, and Pivot subfolders. The next longest import step is importing plot.ps1 which only takes ~98 milliseconds.

In a quick and dirty test where I take all the contents of those subfolders and embed them in the .PSM1 file, the import time drops to only 362 milliseconds. The issue is less noticeable in PowerShell 7 where the current import time is only 1.1 seconds, but "compiling" all the functions into the .PSM1 file still reduces the import time by 20x, down to 53 milliseconds.

I haven't started yet but the idea would be to keep the current project structure, and change the build/CI process so that the version of the module being tested and published is "compiled" by inserting as much as possible into the PSM1 file beforehand.

I use this pattern another module where there are a total of 197 files in the Classes, Public, and Private folders with each function residing in its own file. The published version of the PSM1 file has over 15k lines and imports in ~3.6 seconds.

joshooaj avatar Oct 25 '23 21:10 joshooaj

Thanks for the question. Have thought about that for a while. I'd be open to it.

I'm running Win 11 both version of PS import using -force in ~2 sec.

I have not thought through the scenarios where bugs could arise or debugging efforts.

dfinke avatar Oct 27 '23 02:10 dfinke

Okay, I'll get started! What's your current CI? I see references to Azure DevOps, AppVeyor, and GitHub Actions and I'm not sure if DevOps/AppVeyor are still being used or if those references are stale.

joshooaj avatar Oct 30 '23 15:10 joshooaj

Thanks, really appreaciate it! GitHub Actions is preferred. Az is still enabled.

Note: I'm not sure how I can get this into enough folks hands or forewarn users of this sizable and potential impact. Also, I an not confident of the test coverage to uncover issues.

Additionally. I have other commitments. I may not get to review this for some time.

Given these risks, it could be months before I decide against it or merge the PR.

dfinke avatar Oct 30 '23 16:10 dfinke

Thanks for the notice - I can understand that. If you end up merging any of it, maybe it marks a major version increment to signal that there might be breaking changes (even though the goal is that there aren't).

Looking at the repo a bit closer, there are some other structural changes I think could improve the developer experience, including the addition of a pwsh devcontainer. I don't think any changes I have in mind would make a difference to module users other than to improve the import time, but I know any change like this can have unintended side effects.

joshooaj avatar Oct 30 '23 17:10 joshooaj

I may have gotten a bit carried away... however, the tests pass on all three OS's and the import times are significantly faster when importing the "compiled" copy. I won't send a PR until we've had a chance to chat about all the changes, and to make sure that the CI pipelines and dev experience are 100%, and I understand that may not happen for a while - no worries. I may take a look at some of the open issues or submit a couple bug fixes separately.

Here's a rundown of the changes which you can view at joshooaj/ImportExcel. One of the neat additions is the devcontainer so that you can launch a codespace on GitHub or build in a devcontainer on your local machine if you have docker installed. You should try running the command mkdocs serve from a codespace or devcontainer - it'll start serving a dev copy of the ImportExcel docs site which is (for now) published to GitHub Pages under my repo.

  • Moved all module source into the ImportExcel subfolder.
  • Moved all documentation / examples into the docs subfolder.
  • Added build.ps1 as an entry-point to build the module. Building...
    • Creates an output folder at Output/ImportExcel/version/.
    • Copies the contents of ImportExcel/ to the output folder.
    • Merges the files that are normally dot-sourced into ImportExcel.psm1.
    • Removes unneeded subfolders and files from the output module folder.
  • Added RunTests.ps1 to configure and run Pester.
    • Fixed a test that had unpredictable results because it depended on the OS "system" folder having a minimum number and assortment of files.
    • Added Before.tests.ps1 with a BeforeDiscovery and BeforeAll block where the freshly compiled ImportExcel module can be imported once and used in the rest of the tests without importing in each individual file. Though maybe this should change for better compatibility with the Pester VSCode extension?
  • Fixed a handful of SuppressMessageAttribute attributes attached to empty param blocks outside of any function - this caused an issue when merging standalone PS1 files into the PSM1 file.
  • Changed the way Import-LocalizedData was used to make it cleaner, and easier to support localization in the future.
  • Added .devcontainer/devcontainer.json with Dockerfile to provide a ready-to-run dev experience.
  • Added launch and build configs for VSCode to provide F5 and CTRL+Shift+B support.
  • Added mkdocs.yml and made some minor changes to docs files to improve the appearance of docs built by mkdocs.
  • Added docs.yml GHA workflow for publishing to GH-Pages.
  • Updated GitHub Actions to build/test and publish pester test results, and added a new workflow for building and publishing the docs site from the contents of the docs folder.

joshooaj avatar Oct 31 '23 08:10 joshooaj

Sweet! If you're up for it. I can schedule a zoom chat and you can level set me so I can let it percolate.

dfinke avatar Oct 31 '23 17:10 dfinke

Sounds good, I'm free today and pretty flexible any other day so feel free to shoot an invite over to joshooaj at gmail any time 😎

joshooaj avatar Oct 31 '23 17:10 joshooaj