Subject class creation performance optimization oportunity

[Wei-Jun Wang]

Hi Tigran,

Thanks for your interest on OpenJDK.

I think the timing difference makes sense as the principals are stored in a special set inside the subject so subjectByPassingSet adds each twice and the subjectByAddingToSet only once. As for why we don't just use the pre-populated set as-is since it's already immutable, I think the major reason that we do the wrapping here is to make sure the read-only flag is correctly honored and the correct type of exception is thrown. I admit there is room for optimization when the input is already immutable and the read-only flag is also set, but it’s unclear whether the benefit is worth the complexity.

Best regards,
Weijun

> On Jun 25, 2025, at 03:59, Mkrtchyan, Tigran <tigran.mkrtchyan at desy.de> wrote:
> 
> 
> Dear security-devs,
> 
> While benchmarking various parts of our code, I noticed unexpected (but fully explainable) behavior of how an instance of the Subject class can be created.
> 
> The intuitive one is to create a set of Principals and then pass it to the constructor. However, it turned out that creating an empty subject and populating it with desire principles is much faster:
> 
> 
>    Benchmark                               Mode  Cnt        Score       Error  Units
>    SubjectBenchmark.subjectByAddingToSet  thrpt    9  2061025.436 ± 24807.881  ops/s
>    SubjectBenchmark.subjectByPassingSet   thrpt    9  1341492.178 ± 34701.882  ops/s
> 
> 
> The reason is that the Subject class performs extra checks on the provided set, which first creates a null-clean LinkedList copy of the collection. Makes sense.
> However, `Set#of` returns immutable, null clean sets, so there is no reason for an extra copy.
> 
> 
> Best regards,
>   Tigran.
> 
> 
> The benchmark code:
> 
> ```
> @BenchmarkMode(Mode.Throughput)
> @State(Scope.Benchmark)
> public class SubjectBenchmark {
> 
> 
>    @Benchmark
>    public Subject subjectByAddingToSet() {
>        Subject subject = new Subject();
>        subject.getPrincipals().add(new UnixNumericUserPrincipal(0));
>        subject.getPrincipals().add(new UnixNumericGroupPrincipal(0, true));
>        subject.getPrincipals().add(new UnixNumericGroupPrincipal(1, false));
>        subject.getPrincipals().add(new UnixNumericGroupPrincipal(2, false));
>        subject.getPrincipals().add(new UnixNumericGroupPrincipal(3, false));
>        subject.getPrincipals().add(new UnixNumericGroupPrincipal(4, false));
>        subject.getPrincipals().add(new UnixNumericGroupPrincipal(5, false));
>        subject.getPrincipals().add(new UnixNumericGroupPrincipal(6, false));
>        subject.setReadOnly();
>        return subject;
>    }
> 
> 
>    @Benchmark
>    public Subject subjectByPassingSet() {
> 
>        Principal[] principals = new Principal[]{
>              new UnixNumericUserPrincipal(0),
>              new UnixNumericGroupPrincipal(0, true),
>              new UnixNumericGroupPrincipal(1, false),
>              new UnixNumericGroupPrincipal(2, false),
>              new UnixNumericGroupPrincipal(3, false),
>              new UnixNumericGroupPrincipal(4, false),
>              new UnixNumericGroupPrincipal(5, false),
>              new UnixNumericGroupPrincipal(6, false)
>        };
> 
>        return new Subject(true, Set.of(principals), Set.of(), Set.of());
>    }
> }
> ```