JENA-2354 Make initial size of node caches configurable via StoreParams - Githubissues

apache / jena

Apache Jena

https://jena.apache.org/

Apache License 2.0

1.08k stars 643 forks source link

JENA-2354 Make initial size of node caches configurable via StoreParams #2524

Closed damien-veezoo closed 2 weeks ago

damien-veezoo commented 1 month ago

Issue: https://issues.apache.org/jira/projects/JENA/issues/JENA-2354

Pull request Description:

With TDB 4.9.0, node caches change from Guava to Caffeine and are being assigned an initial capacity of 0.25 * their maximum size, increasing the memory footprint. This PR makes the initial capacity factor configurable via StoreParams.

[ ] Tests are included.
[ ] Documentation change and updates are provided for the Apache Jena website
[x] Commits have been squashed to remove intermediate development commit messages.
[x] Key commit messages start with the issue number (GH-xxxx)

By submitting this pull request, I acknowledge that I am making a contribution to the Apache Software Foundation under the terms and conditions of the Contributor's Agreement.

See the Apache Jena "Contributing" guide.

damien-veezoo commented 2 weeks ago

@afs thanks for reviewing!

The only thing missing is a few tests (one or two ish) in TestStoreParams (TDB1 and TDB2 versions). e.g. a new store_params_05 to check the round trip through JSON.

If I'm not mistaken, store_params_04 already tests the round trip of all store params, that's why I haven't added any new cases initially. But I have now pushed two more explicit test cases wrt. the new store param (for both TDB1 and TDB2).

On the web site, the documentation to update: add to the "per connection" section and the JSON diagram of all settings.

Thanks for the pointer, I have done so and opened the following PR: https://github.com/apache/jena-site/pull/189

damien-veezoo commented 2 weeks ago

@afs I see you have already merged PR https://github.com/apache/jena-site/pull/189. Is there anything more for me to do here or is this PR good to merge as well?

afs commented 2 weeks ago

@damien-veezoo Nothing to do. I ran out of time.

Thanks for addressing the earlier comments.

I'll give it a final check, and if nothing comes up, apply "squash and merge" to get a single commit.

afs commented 2 weeks ago

Done! Thanks.

damien-veezoo commented 2 weeks ago

Thanks @afs!