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

Fix #510 #511

Closed
wants to merge 4 commits into from
Closed

Fix #510 #511

wants to merge 4 commits into from

Conversation

kukulam
Copy link

@kukulam kukulam commented Oct 15, 2020

Solves problem with #510

#hacktoberfest

Screenshot 2020-10-15 at 17 14 02

@kukulam kukulam changed the title #510 Fix #510 Oct 16, 2020
@oleksiyp
Copy link
Collaborator

@kukulam Thanks for the submission! I checked briefly code. Not sure I totally understand why to replace Map with List, need to check it in more detail.

Just wondering if there is any performance degradation here. As seems it is one of the main code paths. Maybe we should use MultiMap or Map<*, List<*>>. Can you please do some measurements on the current test suite to compare or try to provoke slowness.

@kukulam
Copy link
Author

kukulam commented Oct 21, 2020

I will change List into Map<*, List<*>> in the next week and try to make some measurements 🙂

@kukulam
Copy link
Author

kukulam commented Oct 28, 2020

Screenshot 2020-10-28 at 16 51 46

I tried to use kotlinx-benchmark for performance measurements but without success. There are only screens with time from IntelIJ tests result for package: io.mockk.gh.

master
Screenshot 2020-10-28 at 16 40 30

#510
Screenshot 2020-10-28 at 17 01 59

@stale
Copy link

stale bot commented Dec 28, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. If you are sure that this issue is important and should not be marked as stale just ask to put an important label.

@stale stale bot added the stale label Dec 28, 2020
@stale stale bot closed this Jan 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants