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

Find first cold warm dict #2992

Closed
wants to merge 2 commits into from

Conversation

DickBaker
Copy link
Contributor

Issue #2991 and prev comments relate
needs more work but I wanted to demonstrate direction I think EF team should take the perf journey
Hope to hear soon !

Dick Baker added 2 commits December 26, 2020 11:12
2. set Optimise=true (Release build) as Benchmark guidelines
3. append Readkey so results viewable
4. make [some] methods static as analyzer suggestions
5. add .editorconfig file for contributor consistency
FYI I left in DIYMark as debug convenience [drop before merge]
PK driven like most real-world scenarios [AK too wide etc]
request/sequential/random and caching [avoid r-trip] cases
20 methods = 3xRoji, 5xADO, 4xFind, 2xFirst,2xHybrid, 2xObj, 2xDict
perhaps investigate BM RunStrategy, MaxIterationCount, SimpleJob, ShortRunJob
Git stats
EfBench02.* results & my analysis, but needs fuller explanation and Dev guidance
@roji
Copy link
Member

roji commented Jan 8, 2021

@DickBaker what's your specific suggestion here? Are you suggesting directions in which the EF team (or other people) should benchmark to improve EF Core itself, or do you propose merging this PR into the samples themselves, to be part of the docs?

If it's the latter, then as I wrote in #2965 (review), the point of the samples is to provide the simplest possible entry into benchmarking with EF, for new users who have no experience with it. As with #2965, your changes here add significant complexity and would make it more difficult for users to start.

If this is a general perf exploration on your part, with the goal of pointing out areas where EF should be improved, then what we need is a specific point where you think EF needs to be improved/fixed - the additional benchmarking could definitely serve as proof/justification of that.

Basically I'm still trying to understand where you want to go with this...

@DickBaker
Copy link
Contributor Author

I'm happy that EF team are going in the right direction with the architecture & packaging. My itch is that Devs should better understand that there are many ways to kill the cat, and some better than others. So exposing the different ways to invoke EF (eg Find, First, DIY caching etc) so ppl get that there are horses/courses is my motivation. It is always easy to do a level-101 intro but gets increasingly difficult to dive deeper, but it would be a failure if you are not prepared to embark on the next level [sorry I don't buy that Idiots Guide to EF is the only game in town, and to be frightened to drill deeper - this smacks of wokism and lowest common denominator and EF team can do so much better imho].
My attempt this PR was to educate [even entertain?] thinking Devs into good/bad/ugly approaches, and give some quantification rather than simply do x not y if you need z. I want Devs to know enough to be dangerous, and I'm sorry if that doesn't fit the comfortable MS agenda.
In his "Pro .NET Benchmarking" book, Andrey Akinshin explains PDD [p351+] and perf culture [p360+], and I +1 that. IMHO we should encourage enough cultural wisdom in the Dev community even if we could never instill level-400 expertise 100%. I'm not on a mission here but I think MS has authority and integrity to explain enough to instill basic disciplines [I'm sorry the P&P Team was disbanded as they made a huge positive contribution]. Anyway but I wanted to explain my motivations better.
Hope you can endorse some of this and make some use of my feeble contributions !
Best wishes EF team, you have a huge Community following and we are impressed with how such a small team makes such a huge contribution [even if you choose to water down my code & ramblings].

@roji
Copy link
Member

roji commented Jan 25, 2021

Hi again @DickBaker, sorry for not responding earlier to this - it's been busy times over here.

If I'm understanding you correctly, your goal here is to expand our performance docs to go deeper into recommendations for efficient querying/updating etc. I think that's great! As you've noted, the performance docs - as they currently are - are only a very first version and I'm definitely hoping (and planning) that more/deeper content be added. IMHO we have to be careful and strike a balance between going deep - catering for the advanced users - and not overwhelming beginners with information that they don't need to know right away. This is one reason why we have an advanced page, with techniques and analysis that shouldn't (yet) interest newcomers.

More concretely! I'm definitely looking forward to your suggestions. If you're comfortable sending a PR directly against the performance docs with English additions - please do so and I'll be happy to review. Otherwise, if you believe specific recommendations should be there but don't want to do the actual writing, the best way forward is to submit an issue here with a specific suggestion, and if appropriate I can pick that up and make the changes.

Benchmarks are indeed useful for proving your suggested recommendations. In some scenarios it's a good idea to also include the benchmarks in our samples and docs, but it's important for these to be as simplified as possible (while still showing what they need to show).

Does that sound good? In the meantime, would you be OK with closing this PR and #2992 #2991 (comment) as I don't yet see a specific suggestion?

@roji
Copy link
Member

roji commented Feb 6, 2021

@DickBaker I'm going to go ahead and close this for now, as there hasn't been any activity here recently. However, that doesn't mean we're not interest in your contribution - see my thoughts above, if you'd like to contribute please don't hesitate to post back here or open a new PR.

@DickBaker
Copy link
Contributor Author

the PR had specific example codeshowing the various ways of using/misusing EF (and I even tossed in some vanilla ADO for comparison too). Did you not see that ? I found that benchmark.net sometimes couldn't decide how many iterations to run [Andrey writes about multivariant results], so it probably needs some maximum iteration ceiling to ensure runs in acceptable time [I don't care about the decimal point accuracy as the spread between snail-rabbit is so great, and gives strong indicator of what to do/avoid]. Please take another look and my example code as I believe it shines a light into some dark places, and should enlighten to intermediate folk [agree don't want to frighten the children]. Wasn't just you getting bogged down as I see I didn't poll for 13 days (sorry!).

@roji
Copy link
Member

roji commented Feb 8, 2021

@DickBaker I did take a look at the PR, but as I wrote above, if we want to recommend certain best practices (or highlight pitfalls to avoid), then we need to update the actual perf docs with those recommendations - users aren't likely to dig into benchmarks in our samples in search of best practices. We can add benchmarks as support for the recommendation (as I've done in some places), but I believe it's important to keep those benchmarks as simple as possible (even if the recommendations they're supporting is complex); this would keep the benchmarks themselves accessible.

I can also help out and do the actual English modifications to the docs - but I need to know what you think we should add.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants