Improve website conversion time #469
Assignees
Labels
Comments
|
I'm now happy that project level specific code performs well enough. The next step would be to focus on reducing time to convert for a single typical small snippet that'd be used in the Web converter |
GrahamTheCoder
changed the title
|
For some reason the live site is significantly (5x) slower for the trivial sample conversion than running the website locally The local site is slow only for the first request, then fast (with browser cache disabled) |
|
I played around a bit with (unscientifically) Task Manager. Memory for the worker process is negligible, however, the CPU sometimes goes up considerably for the process (up to 30-40%). That's when the slowdown is noticeable (could parallel conversion tax the CPU that much?). I will readily admit that the server is under-powered compared to the previous machine. How about something different (that I wanted to try for a long time): host the API part on Azure Functions? |
|
The parallelization is at the file level, so it shouldn't be kicking in for a single snippet. The memory/recycle values you mentioned by email seem sensible. Might be worth checking event viewer to see if the process is getting restarted due to errors or anything like that. Not sure how well Azure functions deals with the large number/size of assemblies needed in memory to run the thing. Maybe they have some caching to make it performant. I'm certainly up for trying it. |
|
This seems to be even worse recently (especially VB to CS) . I need to get a local repro so I can do some profiling. |
beppe9000
added a commit
to beppe9000/CodeConverter
that referenced
this issue
Jul 27, 2020
* Changelog * Update README.md * Rough outline of dotnet tool that converts all vb in a solution to c# TODO: - MSBuild resolution (probably installing the right packages) - Args to deal with project files - Args to deal with overwriting - Args to deal with different output directory - Args to force skipping nuget restore - Args to force continuing with compile errors Spike of icsharpcode#555 * More args * Handle msbuild properties and projects * Rearrange * SDK version is missing, build against an older one * Allow use of another directory - fixes icsharpcode#217 * Copying fixups * Off topic: Use language names * Use any assemblies hanging around in the app domain too * Pull out project creation * Remove cache for comparison * Improve constructor generation * Parenthesize cast expressions * Run in dot net core and characterize reasonable changes * Use net core types in GlobalImportsStatement * Add missing using * Disable check The versions that come out in .net core are totally different * Try to preserve conversions to char and array for back-compat * Block all overwriting unless comitted to git or forced * Create DirectoryInfoExtensions * Use MSBuildWorkspaceConverter * Make static * Reorder * Don't dispose, let it be used by other tests since it's immutable * Attempts useing VS msbuild so system runtime types resolve * Fix git diff * Avoid yield because it's a linked file * Set AnyCPU to give auto-detection a chance * Try x64 * Run .net core tests * Spike targeting net48 Need to then figure out if we can use a command line argument to launch the net framework version based on a command line option If it works in this form, I can pull all the code into a netstandard library which the tests could reference (and even multi-target themselves if we wanted to test with both runtimes) * Patch in missing behaviour * Split out tons of parts to get around core vs framework nightmare * DotNetTool commands must be dot net core * Dot net core's msbuildlocator uses the net core sdk for workspace creation * The net core sdk can't resolve references for framework projects * Specifically locating msbuild for .net framework throws an error about a missing method - it's not compatible * Shared project * Review: Make some things public I don't really want people from other repos using these since they aren't stable APIs, just helpers. Might need to rethink how to share these * Revert test framework-related changes * Call one from the other * Tell users about this new switch when it inevitably causes pain Would be better off doing some simple detection on the project files probably. The framework version of the exe is actually more flexible since it works for framework and core, but it won't work on other platforms. It may make sense to just try to detect when the framework one is runnable. * Detect environment * Tell people about the keyboard shortcut for copying * Convert omitted delegate parameter types using symbol - fixes icsharpcode#560 * Make dot net core version the main named thing and fix up some warnings * Add a helper script for installing locally * Restore R# required dependencies * Revert nullable test changes * Show errors in tests * Revert readonly span hacks The code will only be run against .net core projects which will support them * Tweak caching to avoid bug in msbuild workspace * Install nuget.build.tasks to fix workspace level errors * Recharacterize wrong behaviour (same as start of branch) Only work in .net core at the moment * Await file writing task * Return same extension by default * Re-set target path after writing * Update readme * Wait for the output to be written * Set working directory in case it helps resolving dependencies * Use latest version over 16.0 - todo: warn if <16.0 is available * Load any assembly, and delete source files that aren't in target * Finalize copy and delegation behaviour * Avoid passing path to git because it's hard to know what format * Log loader exceptions * Give MSBuild locator first go at resolving depenencies + allow others * Don't copy binaries/caches * Check happy path * Tell users when we're ignoring their ancient software * Only limit git diff lines * Write exception message last so user sees it in console * Don't need a stack trace for validation errors that the user can fix * Avoid huge diffs and issues for non git users * Bump version to 8.1.0 * Remove appveyor.yml * Show help on debug install * Tidy up output when git is missing * Async test names to avoid warnings * Fixup most warnings/info items * Remove async suffix * Half package size Turns out they don't need to be content if they're marked as packed. When marked as content it gets included two extra times, and the zip format doesn't seem to compress that well * Move readme and add design notes * Add some future notes * Update README.md * Remove pretty unused code * dlls are binaries * Hack to include dataflow dll for 2017 * Stop binding assembly versions at all, and add version info * 8.1.1 * Update README.md * Update bug_report.md * Deal with array type symbols - fixes icsharpcode#559 * Fix test input compilation * Parenthesize all binary expression - fixes icsharpcode#565 Simplifier will deal with ones that weren't needed * Fix NuGet name * Converting ForEach loop: avoid duplicate variable compilation issue Fixes icsharpcode#558 * Neaten up case when variable is only used within loop * Use var except when casting * Improvements to for loop with missing semantic info - fixes icsharpcode#482 * Loop over enum * Split byref tests into own file * Ref improvements * Lots of WIP for generating a local function * Rename and add xml doc * Manually constructed test case * Widen the hoisting concept * Create local function (a bit eagerly and with a static name) * Less eagerly extract local function * Rename after * Convert if parts in expected order * Handle properties and missing semantic info * Handle other arg types * Handle type mismatches, constants, instance expressions * Release notes * Handle array element references * Pull out an extra temp for element access expressions * Add test for object list * Readability tweaks * Split out, simplify and neaten the output * Extract event tests into own class * Extract property member tests into own class * Extract operator member tests to own class * Use stack so nested types get correct initializers * Tidyups * Add as constructor initializer * Deal with initializers not in the part where constructor is created Fixes icsharpcode#281 * Split arrays with different ranks Part of icsharpcode#544 * Handle MidAssignmentStatement Part of icsharpcode#544 * Convert array type when there's an initializer Part of icsharpcode#544 * Version 8.1.2 * Textual improvements to vsix manifest * One way of solving test race condition * Use confirmed thread safe way of creating workspaces icsharpcode#376 (comment) * Pass the MSBuild default assemblies * Convert to async * Pass through joinable task factory * Rename -> Async * Use non-special hex values * Make switch case labels const where possible Fixes part of icsharpcode#544 * Change the name to avoid icsharpcode#569 * Extract logic * Assign names back to their originals - fixes icsharpcode#569 * Remove explicit cast where possible - fixes icsharpcode#388 * If no project file: Hint at installing the VisualBasic nuget package (for the relevant using statements) - Relates to icsharpcode#388 * Improve performance of VB->CS single file conversion - fixes icsharpcode#469 Avoid converting My namespace which is thrown away anyway * Characterize this oddity * fix typo in readme * Add AsEnumerable in linq "in" clause - fixes icsharpcode#544 * Tidy up string comparison - fixes icsharpcode#500 * Update changelog * Update changelog * Initial copy * Pull in what's needed to get this building in its current form * Move in existing mygroup functionality * Move existing exponentiation logic * Move existing like operator * Fix race condition in GetExitCodeAsync(). After `WaitForExitAsync`, the git process has written all its output to the pipes and terminated. However, it's possible that CodeConverter process has not yet read the data from the pipe, or that `OutputDataReceived` event is still executing after `GetExitCodeAsync()` returns. This can cause an `ObjectDisposedException` if the caller manages to dispose the `StringWriter` before the data is read from the pipe. The `OutputDataReceived` event is called once with `e.Data == null` to signify the end-of-output. Waiting for that prevents the race condition. * Introduce KnownMethod to save inlining 1000s of lines of hardcoding * Modulo should have conversion * Subtle behaviour change: Use bool version rather than late bind * Remove unused using (There's a global import) * Indexers most commonly have 1 arg * Use Array.Resize - fixes icsharpcode#501 * Use C# 7.3 compatible null check and simplify unary negation * Version 8.1.3 * Fix syntax tree issue * Test tweaks * Don't qualify non-static generic names * Don't do any extra qualification for generic names Doesn't cause any negative effects on tests * Fix more predefined type name parsing * Add ref keyword for Array.Resize * Use correct initializer type * Don't use private keyword when already declared private * Nameof expressions contain member access, rather than qualified names * Update comments * Update CHANGELOG.md * Flip condition - a type name should never be a method call * Fix typo * Create delegating method for implementations - fixes icsharpcode#443 - fixes icsharpcode#444 * Improve MyClass tests * Deal with more combinations of explicit / parameters / myclass * Write to temp path - hopefully fixes icsharpcode#578 * Add version * Avoid nullref * Update README.md * Update CodeConverter.csproj * Update description -fixes icsharpcode#577 * Update README.md * Fall back on compilation option * Dedupe and fix syntaxkind * Coerce to string in option compare text for char arrays * Formatting only * Omit conversion * Add test with option compare text true * Cater for nulls too * Pass through comparison and use invocation style * Wrap all cases for case insensitive strings * Deal with case insensitive comparisons * Try to minimize the number of null coalesces * Add self verifying tests * 8.1.4 * Deal with as many combinations of cases as possible * Changelog * Use var pattern to reduce scope of variable - fixes icsharpcode#323 * Only cast when switching on enum * Fix and simplify * Update CONTRIBUTING.md * Fix non-string case regression - fixes icsharpcode#585 * Improve numeric casts - fixes icsharpcode#580 * Set refkind for raise event invocations - fixes icsharpcode#584 * Refactor existing renamer * Rename clashing type members - fixes icsharpcode#420 * Fix string -> enum - fixes icsharpcode#476 * Make comment clearer * Rename explicit method implementations where needed - fixes icsharpcode#492 * Include type info in conversion of default(someType) - fixes icsharpcode#486 * Do a single field exchange, in case someone else exchanges handlers * Factor out class to minimize chance of mixing up two handlers * Report renaming difficulties * Use workspace options * Workaround VS caching issue - fixes icsharpcode#586 * Only set if not already set * Set to the Visual studio JoinableTaskFactory * If there's no symbol we'd need to write a whole different impl * Add to changelog * 8.1.5 * Set file path for text only conversion - fixes icsharpcode#589 * Correct logic for conversion objWithOverloads Is Nothing - fixes icsharpcode#591 * Use object pattern * Correctly use numeric value - fixes icsharpcode#590 * LangVersion 8 * Increase default language version * Switch to "is null" * Correct conversion for equality of overloaded types - fixes icsharpcode#594 * Try to mimimize flakiness due to non-determinism * Improve heuristic for unknown indexer vs method, fix missing params Relates to icsharpcode#595 * Attempted fix for icsharpcode#592 I suspect different visual studio versions are parsing nameof differently. Explicitylt constructing an identifier with the syntax kind may get around this. * 8.1.6 * Revert "Increase default language version" This reverts commit d66da69. * Optional parameters in parameterized properties - fixes icsharpcode#597 * Remove unused and fix typo * Ensure built before conversion Relates to icsharpcode#592 Co-authored-by: GrahamTheCoder <grahamthecoder@gmail.com> Co-authored-by: Christoph Wille <christoph.wille@gmail.com> Co-authored-by: Blubbll <34196146+blubbll@users.noreply.github.com> Co-authored-by: Daniel Grunwald <daniel@danielgrunwald.de>
|
This seems much improved now that the site is using the hosted Azure Function |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Upvote if you want this to happen sooner!
Summary
With recent major additions, one-file-at-a-time performance is definitely slower. The test time has gone from 59 to 87 seconds since 2019-09-24. Obviously it's a bit up and down but there aren't that many more tests and there's an upwards trend.
While single file performance has decreased, I parallelized at the file-level. So I'm hoping that in the case that takes longest - converting whole projects/solutions - that the overall impact is acceptable for now. The snippet converter seems especially slow even for tiny samples so there may be something bad going on there.
Cause
One really big cost is GetCompilationAsync within InitializeSourceAsync.
I expected the performance to decrease when adding things like extra separate passes added for things like normalizing inputs: #386
And when adding conversion of the VB compiler-inlined classes: #387
So some of the slowdown is just due to doing significantly more work in order to get a more accurate conversion.
However, there are certainly other changes which could potentially be optimized, for example in CS->VB: PR #456 introduces a potentially slightly slow operation on a "warm" path: https://github.com/icsharpcode/CodeConverter/pull/456/files#diff-709371109bde62ae9a9082fda70f3a94R192
There are some more big changes coming in a general drive to get VB->CS winforms conversion pretty solid. For example once I've flipped this switch it will go even slower: https://github.com/icsharpcode/CodeConverter/blob/master/ICSharpCode.CodeConverter/CSharp/DocumentExtensions.cs#L45
However, it should allow deleting some other bits of code on hot paths, which will speed things up.
Solution
Do some profiling and optimize as appropriate.
The text was updated successfully, but these errors were encountered: