theolivenbaum commented on pull request #345: URL: https://github.com/apache/lucenenet/pull/345#issuecomment-696811978
@eladmarg : I also find it more readable (looking forward for the `is not null` from C#9, but is object I avoids having to call the the equality operator. @NightOwl888 interesting point... From looking on the generated ASM: https://sharplab.io/#v2:C4LghgzgtgPgAgJgIwFgBQ64GYAEBLAO2AFMAnAMzAGNicBJAWQE86izKacBvAX3U1yIczAMIAbSBBwh6zViQrVaXHHwzrsOOEgBsWhDgAqxCMG7oclnBaubteuABYcAQQAUjFm0WcqASnM0K2D8cjcqHABCAF4cAgBXMTEcADIUnAi8KVEJCCkqf0CQ4q0kAE43ACIACzJiSr8AbhtitWC2qxatQV0tZwAhDzlvDlpCri7gvDCImLjEsQCJoJLggDcwUhwoMCYAI1pYiMhhJnFJZpXVy2m3Hf3aOYSkv2Xrku0KmrqGy/fLDqtLqA6xXbqlBzOERDLwKUYZJaTKy3TLZM65fJUBCIsGrT5VWqkepNJEA4HoHhAA The `is MyClass c` produces a smaller code (which could be better for inlining the function), but I agree it could be a problem if the method is called often with the case where the cast is mostly false and it would be better to have it fail. Is it possible to run the benchmark on this PR to see if there is any measurable impact? @NightOwl888 For the readonly vs structs: had that issue before on our code-base as well :) It also seems like there are plenty of opportunities to reduce allocations by using structs, will check next week when I've some time again if I notice anything on our own usage of Lucene.NET. I'll take a look on the readme - somehow I missed that when I first glanced through it... As we also use Azure Devops, should be fine to configure it on our account there! ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected]
