') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); })(); Add New Test to Track Number of Jitted Methods by ivdiazsa · Pull Request #91235 · dotnet/runtime · GitHub
Skip to content

Add New Test to Track Number of Jitted Methods - #91235

Merged
ivdiazsa merged 24 commits into
dotnet:mainfrom
ivdiazsa:jitted-parts
Sep 25, 2023
Merged

Add New Test to Track Number of Jitted Methods#91235
ivdiazsa merged 24 commits into
dotnet:mainfrom
ivdiazsa:jitted-parts

Conversation

@ivdiazsa

@ivdiazsaivdiazsa commented Aug 28, 2023

Copy link
Copy Markdown
Contributor

Addresses issue #88533. This PR adds a new test that monitors the number of jitted methods in a simple console app. The purpose is to catch big methods regressions faster.

@ivdiazsaivdiazsa added the area-Infrastructure-coreclr Only use for closed issues label Aug 28, 2023
@ghostghost assigned ivdiazsaAug 28, 2023
@ghost

ghost commented Aug 28, 2023

Copy link
Copy Markdown

Tagging subscribers to this area: @hoyosjs
See info in area-owners.md if you want to be subscribed.

Issue Details

Addresses issue #88533. This PR adds a new test that monitors the number of jitted methods in a simple console app. The purpose is to catch big methods regressions faster.

Author:ivdiazsa
Assignees:-
Labels:

area-Infrastructure-coreclr

Milestone:-

@ivdiazsaivdiazsa added this to the 9.0.0 milestone Aug 28, 2023
@ivdiazsaivdiazsa linked an issue Aug 28, 2023 that may be closed by this pull request
R2R: Add a CI or perf validation for # of methods jitted #88533
Closed
@ivdiazsa

ivdiazsa commented Aug 28, 2023

Copy link
Copy Markdown
ContributorAuthor

/azp run runtime-coreclr outerloop

@azure-pipelines

azure-pipelinesBot commented Aug 28, 2023

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@ivdiazsa
ivdiazsa marked this pull request as ready for review August 29, 2023 16:33
@ivdiazsa
ivdiazsa requested a review from trylekAugust 29, 2023 16:33
@ivdiazsa

ivdiazsa commented Aug 29, 2023

Copy link
Copy Markdown
ContributorAuthor

@trylek I had problems with my old fork and had to make a new one, hence a new PR. PTAL

@ivdiazsa
EXPERIMENTAL CHANGE ONLY: Added a command to print the whole directory
b565e74
tree after running the tests, and adjusted the pipelines
configuration to only run that one.
@dotnetdotnet deleted a comment from azure-pipelinesBotAug 29, 2023
@ivdiazsa
experimental change only: Discovered the jobs have dependencies on
2317a4b
other jobs. Disabled the runtime pipeline entirely, and only kept the
relevant outerloop platforms for my change to save resources.
@ivdiazsa

ivdiazsa commented Aug 29, 2023

Copy link
Copy Markdown
ContributorAuthor

/azp run runtime-coreclr outerloop

@azure-pipelines

azure-pipelinesBot commented Aug 29, 2023

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@ivdiazsa
EXPERIMENT-ONLY CHANGE: Added the 'ls -R' directly to the Helix Post
caf11e8
Commands, and disabled the dev-innerloop pipeline to further speed up
this testing and debugging process.
@ivdiazsa
Merge branch 'main' into jitted-parts
fda5625
@dotnetdotnet deleted a comment from azure-pipelinesBotAug 30, 2023
@ivdiazsa

ivdiazsa commented Aug 30, 2023

Copy link
Copy Markdown
ContributorAuthor

/azp run runtime-coreclr outerloop

@azure-pipelines

azure-pipelinesBot commented Aug 30, 2023

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@dotnetdotnet deleted a comment from azure-pipelinesBotAug 30, 2023
@ivdiazsa

ivdiazsa commented Aug 30, 2023

Copy link
Copy Markdown
ContributorAuthor

/azp run runtime-coreclr outerloop

@azure-pipelines

azure-pipelinesBot commented Aug 30, 2023

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@ivdiazsa
ivdiazsa marked this pull request as draft August 31, 2023 18:35
@dotnetdotnet deleted a comment from azure-pipelinesBotAug 31, 2023
@dotnetdotnet deleted a comment from azure-pipelinesBotSep 11, 2023
@ivdiazsa

ivdiazsa commented Sep 11, 2023

Copy link
Copy Markdown
ContributorAuthor

/azp run runtime-coreclr outerloop

@azure-pipelines

azure-pipelinesBot commented Sep 11, 2023

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@trylek

trylek commented Sep 11, 2023

Copy link
Copy Markdown
Member

Please do make sure to remove all instrumentation from the yml scripts, most notably I don't see why this PR should be changing eng/pipelines/ci.yml.

@ivdiazsa

ivdiazsa commented Sep 12, 2023

Copy link
Copy Markdown
ContributorAuthor

Please do make sure to remove all instrumentation from the yml scripts, most notably I don't see why this PR should be changing eng/pipelines/ci.yml.

Good catch. Those changes must have filtered at some point during one of the rebases with main. Will remove them.

@ivdiazsa

ivdiazsa commented Sep 12, 2023

Copy link
Copy Markdown
ContributorAuthor

Sigh... Windows is causing problems again... Investigating those failures now.

jkotas
jkotas reviewed Sep 12, 2023
View reviewed changes
Comment threadsrc/tests/readytorun/JittedMethodsCountingTest/JittedMethodsCountingTest.cs Outdated

// For adding any new apps for this test, make sure they return a negative
// number when failing.
int appResult = RunHelloWorldApp(appName);

@jkotasjkotasSep 12, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The setup with the app running in a separate process made sense when the test used JIT logging to get the method count.

This setup is overkill now that the test just calls GetCompiledMethodCount and checks its value. Would it be better to turn this into an ordinary single process test that just calls GetCompiledMethodCount and checks its value (and skipping the check in specific environments)?

@ivdiazsaivdiazsaSep 12, 2023•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, this makes a lot of sense. I just tested it on my local machine and I'm getting 4 jitted methods instead of the 1 I used to get with the solo HelloWorld app. Since the number of jitted methods is very similar, does it make sense to you to do it like this instead @trylek?

@ivdiazsa
Changed the test to not require an external app anymore, and fixed
adbb0b2
some small bad merges in the CI files when rebasing.
@ivdiazsa

ivdiazsa commented Sep 12, 2023

Copy link
Copy Markdown
ContributorAuthor

/azp run runtime-coreclr outerloop

@azure-pipelines

azure-pipelinesBot commented Sep 12, 2023

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@build-analysisbuild-analysisBot mentioned this pull request Sep 13, 2023
Closed
@dotnetdotnet deleted a comment from azure-pipelinesBotSep 18, 2023
@dotnetdotnet deleted a comment from azure-pipelinesBotSep 18, 2023
@ivdiazsa

ivdiazsa commented Sep 18, 2023

Copy link
Copy Markdown
ContributorAuthor

/azp run runtime-coreclr outerloop

@azure-pipelines

azure-pipelinesBot commented Sep 18, 2023

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@build-analysisbuild-analysisBot mentioned this pull request Sep 19, 2023
Open
@ivdiazsa
Due to how some machines are set up, it is actually possible for a
d9cb394
dotnet app to be fully prejitted, and therefore show 0 jitted methods,
so added that case to the test.
@ivdiazsa

ivdiazsa commented Sep 21, 2023

Copy link
Copy Markdown
ContributorAuthor

/azp run runtime-coreclr outerloop

@azure-pipelines

azure-pipelinesBot commented Sep 21, 2023

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

This was referenced Sep 21, 2023
Closed
Closed
@ivdiazsa

ivdiazsa commented Sep 25, 2023

Copy link
Copy Markdown
ContributorAuthor

Failures are unrelated. Merging this now

@ivdiazsa
ivdiazsa merged commit dc5225d into dotnet:mainSep 25, 2023
jkotas
jkotas reviewed Sep 25, 2023
View reviewed changes
Comment threadsrc/coreclr/vm/jitinterface.cpp
{
FCALL_CONTRACT;

// WIP-DEBUG-ONLY change: Windows is causing problems in the CI, but can't

@jkotasjkotasSep 25, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This WIP code needs to be reverted!

@ivdiazsaivdiazsaSep 25, 2023

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I forgot! Good catch, Jan. I will submit a PR fixing that right away.

@ivdiazsaivdiazsa mentioned this pull request Sep 25, 2023
Merged
@jkotasjkotas mentioned this pull request Oct 7, 2023
Closed
@ghostghost locked as resolved and limited conversation to collaborators Oct 26, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jkotasjkotasjkotas left review comments
@trylektrylektrylek approved these changes

Assignees

@ivdiazsaivdiazsa

Labels

area-Infrastructure-coreclrOnly use for closed issues

Projects

Infrastructure Backlog
Status: Closed

Milestone

9.0.0

Development

Successfully merging this pull request may close these issues.

R2R: Add a CI or perf validation for # of methods jitted

3 participants

@ivdiazsa@trylek@jkotas