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

Remove usage of lambdas in SegmentMerger? [LUCENE-10180] #11217

Closed
asfimport opened this issue Oct 15, 2021 · 10 comments
Closed

Remove usage of lambdas in SegmentMerger? [LUCENE-10180] #11217

asfimport opened this issue Oct 15, 2021 · 10 comments

Comments

@asfimport
Copy link

SegmentMerger now uses lambdas to share the logic around logging merging times for all file formats.

One problem is that these lambdas get auto-generated names, and it makes it harder to work with profilers since things that should logically end up in the same sub tree end up in different sub trees because two instances of the same lambda get different names.


Migrated from LUCENE-10180 by Adrien Grand (@jpountz), resolved Oct 19 2021
Attachments: profile.png

@asfimport
Copy link
Author

Adrien Grand (@jpountz) (migrated from JIRA)

profile.png

Here is a profile generated by async-profiler to show what I'm talking about. There are two separate sub trees for points merges under SegmentMerger#merge because we get two lambdas that have different auto-generated names.

@asfimport
Copy link
Author

Michael Sokolov (@msokolov) (migrated from JIRA)

Thanks for digging - does the proposed solution (function pointers) make the profiles more consistent? I expect it would ...

@asfimport
Copy link
Author

Vigya Sharma (@vigyasharma) (migrated from JIRA)

I would like to work on this issue and get more involved with merge related functionality. Might need some help as I'm new here.

Is the idea here to use a function pointer (similar to the one used for mergeTermVectors) for each of the merge tasks?

@asfimport
Copy link
Author

Michael Sokolov (@msokolov) (migrated from JIRA)

It looks as if @jpountz posted a PR: https://github.com/apache/lucene/pull/385. I'm not sure why it didn't get linked here automatically

@asfimport
Copy link
Author

asfimport commented Oct 19, 2021

Adrien Grand (@jpountz) (migrated from JIRA)

I'm not sure either, I don't think I did anything special so that this PR would not get linked here. Sorry @vigyasharma. If you're looking for an easy issue to get started, I could recommend this one: #11122, though it's not related to merging.

@asfimport
Copy link
Author

Adrien Grand (@jpountz) (migrated from JIRA)

does the proposed solution (function pointers) make the profiles more consistent?

Yes it does, since the method ref is always given its actual name in profiles.

@asfimport
Copy link
Author

ASF subversion and git services (migrated from JIRA)

Commit 1448e47 in lucene's branch refs/heads/main from Adrien Grand
https://gitbox.apache.org/repos/asf?p=lucene.git;h=1448e47

LUCENE-10180: Avoid using lambdas in SegmentMerger. (#385)

@asfimport
Copy link
Author

Uwe Schindler (@uschindler) (migrated from JIRA)

does the proposed solution (function pointers) make the profiles more consistent?

Yes it does, since the method ref is always given its actual name in profiles.

Background for @msokolov: Lambdas can't be compiled to bytecode without creating a method out of it (and then make a reference to the same type of method reference syntax in the lambda bootstrap invokedynamic). So a -> foobar(a) will generate a static or virtual method (depending on if access to "this" is needed) named lambda$XY(a) with the body return foobar(a). This is of course not needed but you always see the lambda method in the stack traces. So to better allow to see where something happens in "simple cases" (does not work in complex chains with Java streams): Avoid lambdas and add the bodies as methods. But always look at signatures and always prefer a method reference anywhere in code if a lambda that only calls another method with exact same parameter (with or without "this" capture).

@asfimport
Copy link
Author

ASF subversion and git services (migrated from JIRA)

Commit 1448e47 in lucene's branch refs/heads/hnsw from Adrien Grand
https://gitbox.apache.org/repos/asf?p=lucene.git;h=1448e47

LUCENE-10180: Avoid using lambdas in SegmentMerger. (#385)

@asfimport
Copy link
Author

Adrien Grand (@jpountz) (migrated from JIRA)

Closing after the 9.0.0 release

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

No branches or pull requests

1 participant