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.mkrtch...@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());
> }
> }
> ```
