Re: Race condition in OnHeapHnswGraph

Viliam ġurina <[email protected]> Fri, 16 Jan 2026 09:41:25 +0100
Newsgroups gmane.comp.jakarta.lucene.user
Message-ID <CAO=iB8KTVk9mXxBP_CEzLsqPzG_5bqyujydh9Xbke+sqJrEs_w@mail.gmail.com>
--000000000000a2487f06487d50a3
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

I'd like to do the PR, but could you please confirm to me that it's safe to
use non-volatile field? I believe this should be the case.

Viliam

On Thu, Jan 15, 2026 at 12:25=E2=80=AFAM Michael Sokolov <[email protected]=
m> wrote:

> yeah this looks silly: do you want to open a PR to fix?
>
> On Mon, Jan 12, 2026 at 6:39=E2=80=AFAM Viliam =C4=8Eurina <viliam.durina=
@gmail.com>
> wrote:
> >
> > Hi all,
> >
> > I'm looking at `OnHeapHnswGraph` code and noticed that the volatile fie=
ld
> > `graphRamBytesUsed` is modified in `addNode` in a racy way:
> >
> >   long bytesUsed =3D graphRamBytesUsed;
> >   graphRamBytesUsed =3D bytesUsed + l;
> >
> > This is equivalent to `graphRamBytesUsed +=3D l`.
> >
> > This code is susceptible to lost update due to non-atomic
> read-modify-write
> > operation.
> >
> > I guess the it's not really a problem, because this code is in fact
> > single-threaded when documents are added to the index. It might be
> > concurrent during merging, but then `ramBytesUsed()` isn't called, and
> it's
> > a wasted work.
> >
> > If the above assumption is correct, then this field should not be
> volatile
> > to improve performance. If it's not, it should be replaced with
> > `AtomicLong` or `LongAdder`.
> >
> > Viliam
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [email protected]
> For additional commands, e-mail: [email protected]
>
>

--000000000000a2487f06487d50a3--