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.

Sign up for GitHub

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

Jump to bottom

Review J2N read-only collections performance #984

Open
1 task done
paulirwin opened this issue Oct 22, 2024 · 1 comment
Open
1 task done

Review J2N read-only collections performance #984

paulirwin opened this issue Oct 22, 2024 · 1 comment
Assignees
@paulirwin
Labels
good-first-issue Good for newcomers is:task A chore to be done pri:normal
Milestone
4.8.0-beta00018

Comments

@paulirwin
Copy link
Contributor

Is there an existing issue for this?

  • I have searched the existing issues

Task description

Split from #928:

Originally posted by @NightOwl888:

Each of the read only collections in J2N has a "Default" mode, which kicks in if the type being passed in is a J2N collection, where the rules of structural equality are defined. If it is a BCL or 3rd party collection, it will use "Aggressive" mode, which tries to patch the collection externally with the same behavior. But "Aggressive" mode is very costly in terms of performance.

... we should only use "Aggressive" mode if we are dealing with a user-supplied collection that we have no control over. However, if we have control over the concrete collection type that is used (or default), we should always ensure that a J2N collection is used for each of these calls so we don't downgrade to "Aggressive" mode. So, it would be a good idea to review to make sure we didn't swap to any BCL collections where it could matter.

Review usage of J2N read-only collections to ensure they use the Default mode.
@paulirwin paulirwin added up-for-grabs This issue is open to be worked on by anyone good-first-issue Good for newcomers is:task A chore to be done labels Oct 22, 2024
@paulirwin paulirwin added this to the 4.8.0 milestone Oct 22, 2024
This was referenced Oct 22, 2024
Open
Merged
@paulirwin paulirwin modified the milestones: 4.8.0, 4.8.0-beta00018 Nov 18, 2024
@paulirwin paulirwin self-assigned this Jan 7, 2025
@paulirwin paulirwin removed the up-for-grabs This issue is open to be worked on by anyone label Jan 7, 2025
@paulirwin
Copy link
Contributor Author

@NightOwl888 I'm reviewing the J2N read-only collections code to make sure I understand, and I'm not sure how this is true: "Each of the read only collections in J2N has a "Default" mode, which kicks in if the type being passed in is a J2N collection, where the rules of structural equality are defined."

It appears to me like these go into Default mode if the generic type arguments are a value type, IStructuralEquatable, or string, regardless of the concrete collection type passed in: https://github.com/NightOwl888/J2N/blob/8845a696e2cd3e2ca0752823c779c72aa7f0e5a1/src/J2N/Collections/ObjectModel/ReadOnlyList.cs#L67

This does not seem to be dependent on the collection being a J2N collection. It appears as if passing i.e. a System.Collections.Generic.List<string> would meet that criteria for Default. Let me know if I'm missing something. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@paulirwin paulirwin

Labels
good-first-issue Good for newcomers is:task A chore to be done pri:normal
Projects
None yet
Milestone
4.8.0-beta00018
Development

No branches or pull requests
1 participant
@paulirwin