azurefieldnotes / ReportHTML

47 stars 21 forks source link

Performance Improvement #10

Open GitHubBJW opened 6 years ago

GitHubBJW commented 6 years ago

Greetings, In the ReportHTML.psm1, I noticed the code spent a long time on the line 227: ForEach-Object{$ScriptHeaders += $_ + "rn" } I could be misunderstanding why the carriage return and line feeds are being added. If those aren't necessary it is also not necessary to cycle through each line of those .js files. I modified the .psm1 file locally to the following: $ScriptHeaders += Get-Content $ScriptFile -ReadCount 0 and performance was greatly improved. It didn't break the report I was generating, and I don't want to be presumptuous and assume that it won't break something else. Please consider this change if it works in the larger context of the code. Thank you. Brian

spaelling commented 5 years ago

You can also replace https://github.com/azurefieldnotes/ReportHTML/blob/master/ReportHTML/ReportHTML.psm1#L228-L236

with below code which does the exact same thing, just ~100 times faster.

    $ScriptHeaders = New-Object System.Collections.Generic.List[System.Object]
    foreach ($ScriptFile in $ScriptFiles) {
        Write-Verbose "Generating Script Header from $ScriptFile"
        $ScriptHeaders.Add(("`r`n" + '<script type="text/javascript">  '+ "`r`n"))
        $Content = Get-Content $ScriptFile
        foreach ($item in $Content) {
            $ScriptHeaders.Add("$item`r`n")
        }
        $ScriptHeaders.Add('</script> ')
    }
    $ScriptHeaders = $ScriptHeaders.ToArray()
lucidqdreams commented 5 years ago

Sorry Brian, I missed the bug git update in my email. OMG! Thanks so much for finding this. Updating code now (1.4.1.2) and will release ASAP.

spaelling commented 5 years ago

just made a pull request you can use

also commented out some duplicate code (could be duplicate for a reason)

spaelling commented 5 years ago

Not sure what happened with the merge, but the code is not in the masterbranch

binf commented 5 years ago

Wish i would have seen that quicker but while trying to look for project contact i stumbled uppon this issue, here is my variation (#2) since the first one using regex was broken.

The simplest way i found was this one and it is really effective.

Function Get-HTMLJavaScripts
{
<#
    .SYNOPSIS
        Get's Script File from module directory
#>
    [CmdletBinding()]
    param 
    (
        [Parameter(Mandatory=$false)]
        [String]
        $ScriptPath
    )
    if([String]::IsNullOrEmpty($ScriptPath))
    {
        if([string]::IsNullOrEmpty($PSScriptRoot))
        {
            $ScriptPath=$PSCmdlet.SessionState.Path.CurrentLocation.Path
        }
        else
        {
            $ScriptPath=$PSScriptRoot
        } 
    }
    Write-Verbose "Retrieving *.js from $ScriptPath"
    $ScriptFiles = @((get-childitem $ScriptPath -Filter '*.js' ).fullname)

    $ScriptHeaders = @()
    foreach ($ScriptFile in $ScriptFiles) {
        Write-Verbose "Generating Script Header from $ScriptFile"
        $ScriptHeaders += "`r`n" + '<script type="text/javascript">  '+ "`r`n" 
        $ScriptHeaders += Get-Content -Encoding String $ScriptFile -Delimiter "`n"
        $ScriptHeaders += '</script> ';
    }
    Write-Output $ScriptHeaders
}
M1kep commented 5 years ago

@lucidqdreams It looks like the merge didn't actually bring the edits over for some reason.

They seem to make a huge performance boost when I add them manually

PrzemyslawKlys commented 5 years ago

I know it's a bit impolite to bomb another repo, but thought I would let you guys know. I've forked ReportHTML a while back, renamed it to PSWriteHTML and rewrote most if not all of the code. @lucidqdreams I really do hope you don't mind this but I've actually heavily simplified ReportHTML and even started 2 supportive products based on it (Dashimo and Statusimo). I've updated all the libraries, rewritten code generation, fixed some issues and expanded upon your ideas. It's still work in progress. I'm migrating ChartJS to ApexCharts as I prefer their visual part, I also added lots of options to Datatables including Conditional Formatting. Feel free to remove this if you feel I'm violating your rules. I didn't intend to. For PSWriteHTML things still change dynamically as I've not decided on final function naming but Dashimo has this simplified.

Here's an idea to show you how it works:


$Process = Get-Process | Select-Object -First 30
$Process1 = Get-Process | Select-Object -First 5
$Process2 = Get-Process | Select-Object -First 10
$Process3 = Get-Process | Select-Object -First 10

Dashboard -Name 'Dashimo Test' -FilePath $PSScriptRoot\DashboardEasy.html -Show {
    Tab -Name 'First tab' {
        Section -Name 'Test' {
            Table -DataTable $Process
        }
        Section -Name 'Test2' {
            Panel {
                Table -DataTable $Process1
            }
            Panel {
                Table -DataTable $Process1
            }
        }
        Section -Name 'Test3' {
            Table -DataTable $Process -DefaultSortColumn 'Id'
        }
    }
    Tab -Name 'second tab' {
        Panel {
            Table -DataTable $Process2
        }
        Panel {
            Table -DataTable $Process2
        }
        Panel {
            Table -DataTable $Process3 -DefaultSortIndex 4
        }
    }
}
lucidqdreams commented 5 years ago

I have always been open to collaboration as this was always a side project I worked on when I had the time or when needed it for work. Currently, I am swamped, lost in the day job so haven't had time to work in peoples code and fixes, unfortunately. It came about because something was missing. I was actually hoping to see people publish reusable reports. I had published a few prototypes. I wasn't aware of apexcharts seems like an interesting replacement. Not sure if it was around when I first added chart JS. It was originally written a very long time ago so PowerShell has changed a lot so a rewrite probably a good thing. Its public, so what you do is fine. That said I have always given credit to all the people that helped even a little bit. A quick search of the internet for statusimo and I find https://evotec.xyz/meet-statusimo-powershell-generated-status-page/ which says "after a few days I had a prototype that didn't require much work to generate". And Bradley Wyatt in dashimo seems to get some inspired by credit here https://evotec.xyz/meet-dashimo-powershell-generated-dashboard/. Obviously, you know how much code you wrote, rewrote being how much actually made up or just reworked. It's Up to you, but I would and always have given credit to those contributions and hard work of others https://www.linkedin.com/in/matthewquickenden/

PrzemyslawKlys commented 5 years ago

I've never hidden that Dashimo and Statusimo both come from PSWriteHTML which is a fork of ReportHTML (and can be seen here https://github.com/EvotecIT/PSWriteHTML). I never intended to remove that connection so you get all the credit in there with all your commits and all.

I do mention ReportHTML in most places but I'll give you a direct named credit on your LinkedIn account as per your request. Actually, Statusimo is mostly my work but still based on some ideas from ReportHTML.

As for the credits to Brad, well Brad showed me how to use ReportHTML on his website https://www.thelazyadministrator.com/2018/12/04/get-an-active-directory-interactive-html-report-with-powershell/ that's why he got the credit. Because if I wouldn't see that I wouldn't want to rewrite ReportHTML.

I do believe people should get recognized for their work. I'll make sure to make it clear in most of materials on next update.

lucidqdreams commented 5 years ago

I do appreciate that. It has been a long road to get here and many iterations and learnings through several versions of Powershell as well, I actually think it started in V1. The open source community is a collaborative one and recognition is free to give, great to receive and doesn't invalidate any of the work you do unless its a copy paste rename job. I just wanted something to help turn PowerShell data into something I can send people. If you can help me do my job better, that's great. Its also helps if you go looking for another job and people ask "what have you done?", I wrote a thing, inspired someone else... That's the stuff people are looking for if you know what I mean.

PrzemyslawKlys commented 5 years ago

I do like helping and my motivation for Dashimo / PSWriteHTMl and overall my website with all the modules and blog posts (https://evotec.xyz/hub) is my way of giving back to a community. I wouldn't know what I know without all the things I did read on the Internet, or in this case, I wouldn't be able to create this without having your code. While I have probably not left a single line of code untouched in PSWriteHTML it allowed me to slowly throw away your code and put new code. Things have changed over the years and my code only works in PS 5.1+ while yours can probably run PS 2.0 or so. I've learned PowerShell over the last 2 years and things that worked for me 1 year ago now seem outdated.

I do hope to see one day seeing you using Dashimo as the new version expands even further with TableConditionalFormatting https://evotec.xyz/dashimo-easy-table-conditional-formatting-and-more/

I'll make sure to give credit you deserve.

lucidqdreams commented 5 years ago

And they all coded happily ever after :)

Thank you.

PrzemyslawKlys commented 5 years ago

Added credits for you on both pages:

Also added links to ReportHTML.

lucidqdreams commented 5 years ago

Very kind. I am very much looking forward to downloading it and having a play around. First, unfortunately, the day job and the looming POC demo.