Consider detaching entity while consuming it from Stream

[quaff]

The main purpose of Stream is avoiding return a large List which may lead to OOM, but before the transaction finished, the entities still exist in persistence context and cannot be recycled by GC, we need to evict entity manually after it be consumed. Here is a test case verify that:

package stream;

import java.util.stream.Stream;

import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.TestInstance;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.autoconfigure.domain.EntityScan;
import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest;
import org.springframework.data.domain.Sort;
import org.springframework.data.jpa.domain.AbstractPersistable;
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.JpaSpecificationExecutor;
import org.springframework.data.jpa.repository.config.EnableJpaRepositories;
import org.springframework.test.context.ContextConfiguration;
import org.springframework.test.context.TestPropertySource;

import jakarta.persistence.Entity;
import jakarta.persistence.EntityManager;

@TestInstance(TestInstance.Lifecycle.PER_CLASS)
@DataJpaTest(showSql = false)
@TestPropertySource(properties = "spring.jpa.properties.hibernate.jdbc.fetch_size=100")
@EnableJpaRepositories(basePackageClasses = StreamTests.TestEntityRepository.class, considerNestedRepositories = true)
@EntityScan(basePackageClasses = StreamTests.TestEntity.class)
@ContextConfiguration(classes = StreamTests.class)
class StreamTests {

	@Autowired
	TestEntityRepository repository;

	@Autowired
	EntityManager entityManager;

	@BeforeAll
	void prepare() {
		int size = 100000;
		for (int i = 0; i < size; i++) {
			TestEntity entity = new TestEntity();
			this.repository.save(entity);
		}
	}

	@ParameterizedTest
	@ValueSource(booleans = { false, true })
	void testStreamMemoryUsage(boolean detaching) {
		long usedMemory = usedMemory();
		Stream<TestEntity> stream = repository.stream();
		if (detaching) {
			stream = stream.peek(entityManager::detach);
		}

		stream.forEach(entity -> {
			// TODO do something with entity
		});

		System.out.println(
				"Memory usage increased " + (detaching ? "with	" : "without	") + " detaching: " + (usedMemory() - usedMemory));
	}

	private long usedMemory() {
		System.gc();
		return Runtime.getRuntime().totalMemory() - Runtime.getRuntime().freeMemory();
	}

	interface TestEntityRepository extends JpaRepository<TestEntity, Long>, JpaSpecificationExecutor<TestEntity> {

		default Stream<TestEntity> stream() {
			return findBy((root, query, cb) -> null, q -> q.sortBy(Sort.by("id"))).stream();
		}
	}

	@Entity
	static class TestEntity extends AbstractPersistable<Long> {

	}

}

it will output like this:

Memory usage increased without	 detaching: 3060128
Memory usage increased with	 detaching: 302464

Spring Data JPA could return Stream with .peek(entityManager::detach).
Or use a StatelessSession to fetch stream, It's Hibernate specific, I think there is an equivalent with EclipseLink, we should migrate if standard StatelessEntityManager is out.

[mp911de]

That isn't going to happen as detaching entities breaks how JPA works. Registering an aspect on JPA's Query to post-process getResultList() and other methods producing results if you want to detach entities could be reasonable. That would however have to happen in a component closer to Spring Framework.

[quaff]

That isn't going to happen as detaching entities breaks how JPA works. Registering an aspect on JPA's Query to post-process getResultList() and other methods producing results if you want to detach entities could be reasonable. That would however have to happen in a component closer to Spring Framework.

Could you explain why detaching entities breaks how JPA works and how to register an aspect on Query to post-process getResultList()?

[mp911de]

Probably stating the obvious, JPA expects attached entities to work as designed (dirty tracking, lazy loading, persisting on transactional commit)

[quaff]

Probably stating the obvious, JPA expects attached entities to work as designed (dirty tracking, lazy loading, persisting on transactional commit)

I think it's worthy to treat specially, because the purpose of Stream is for fetching large ScrollableResults, we should do that in readonly transaction to prevent OOM, and If someone insist on that, they can merge entity manually.
It's a reasonable tradeoff, please rethink.

[mp911de]

If you want a reasonable fix, please fix the JPA spec.

[quaff]

If you want a reasonable fix, please fix the JPA spec.

I'm not saying something is wrong have to fix, I'm proposing an enhancement, It's kind of contributing, I apologize if made noise here.