Fixed incorrect docs PersistentPropertyPath and optimized sublist

[mipo256]

Fixed incorrect docs in PersistentPropertyPath#isBasePathOf and optimized getExtensionForBaseOf sublisting

[odrotbohm]

Mixing up different things in a single PR makes reviewing them pretty cumbersome because we might decide we like one change but not the other. Can you elaborate what you mean by "optimize"? What exactly? For whom? You also introduce new API, without a clear reason why that's needed.

[mipo256]

Let me explain then one by one

1. Optimization - getExtensionForBaseOf method previously used isBasePathOf. This method iterated over the the properties path and checks whenever the current path is base for passed. And then we just reiterate over the whole properties, when we can just put the remaining part. You can check the code yourself, it is just skipping the first N elements and only then adds the remaining - we can just do the sublist. We do not spend time now to reiterate on the part we have already checked
2. Optimization for whom - for any spring-data project, for spring-data-jdbc in particular.
3. Introduction of the new API is arguable, I agree - I can make method private - I though it was reasonable to abstract the logic from isBasePathOf. What it dose - it actually does 2 things - it tries to find the common part and then ensures that the common part is the same as this. I though we can separate this 2 things, and I decided to make it public, maybe it will be needed by projects. I can remove it, if you think it make sense, but in general method isBasePathOf does 2 things

[odrotbohm]

Thanks for the detailed explanation. Have you run any JMH benchmarks to measure the difference in performance and memory consumption. While I totally see that we can just use ….subList(…) in getExtensionForBaseOf(…), I'm not sure whether the change in getCommonBaseWith(…) really improves the situation. Your suggestion creates both a temporary List, a reference DefaultPersistentPropertyPath from that for the latter to be piped into an equals(…) comparison which will in turn again have to traverse both internal collections. Superficially, that doesn't sound more efficient to me that just traversing an iterator (which your variant has to as well) and returning the result immediately.

[mipo256]

About JMH bnchmarks - I will run against the getExtensionForBaseOf method, thanks for suggestion.
I am not trying to say that getCommonBaseWith(...) is anyhow better :) I have created it just to separate the responsibility - the actual check logic (condition itself) and search for the common base.

Regarding getExtensionForBaseOf - it is supposed to be faster I wager. After making sure that passed path is the base for current, there was such logic, which again traverses the proeprties.

		List<P> result = new ArrayList<>();
		Iterator<P> iterator = iterator();

		for (int i = 0; i < base.getLength(); i++) {
			iterator.next();
		}

		while (iterator.hasNext()) {
			result.add(iterator.next());
		}

I have just changed it to subList(), as it will only create an instance of SubList (in the example above there still would be ArrayList in its place) and will not traverse the entire properties obejct again.

But in any case, I perfectly agree that we should run JMH benchmark. Unfortunately I have not found infrastructure for this in spring-data-commons in general. Am I supposed to just test it on my machine without pushing anything to git?

[odrotbohm]

We keep a loose collection of benchmarks here. Would be nice if you just some in a PR over there so that we can run those against the current codebase and your suggestions to see what difference it makes. Note that right now, I'd only be interested in the performance of getExtensionForBaseOf(…) and isBasePathOf(…) are these are the only methods currently in use in Spring Data modules.

[mipo256]

Hello @odrotbohm !
I have written a GitHub repository to benchmark the changes, see here: https://github.com/Mikhail2048/spring-data-commons-benchmark-2782

It would be great if you will try to benchmark on your hardware on your side. It is pretty simple, you can do mvn clean test and the benchmarks will be executed. Also, make sure to change the version of spring-data-commons in the pom: https://github.com/Mikhail2048/spring-data-commons-benchmark-2782/blob/master/pom.xml#L22

I have current version build and I switched to my branch, build spring-data-commons locally from my branch put the version into the pom.xml.

I have received the following results.
This is the current state:

This is my changes:

As you can see, there is almost no difference. So, take a look at PR again please, and let's decide what to do. I think I can just leave the documentation changes.

Thanks in advance!