Skip to content

Conversation

@edwintorok
Copy link
Contributor

To debug a potential memory leak, not intended for merging yet.

forget_client leaks a thread, it doesn't actually tell it to finish or join it,
just sets a global variable to None.
We do not actually daemonize anymore (we use systemd), so wrap the
forget_client call in an `if !daemon'.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
@edwintorok
Copy link
Contributor Author

#772 the trunk version is here

VM.import_metadata_async creates a task and ignores it.
This causes the tasks to accumulate in the tasks and updates list,
causing high CPU and memory usage.

Set a flag on these tasks to automatically destroy them when they finish
(i.e. complete or fail). If the task is already finished at the time we
set the flag (there could be a race), then immediately destroy it.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
… instead of Uutf.{encode,decoder}

Uutf.encoder allocates a 64k buffer internally (in addition to our 1k buffer).
Use Uutf.Buffer to write utf8 chars directly into our buffer instead.
Also size the buffer based on initial string size (it can grow if needed).

This should reduce allocation rate.

Thanks to a hint from https://discuss.ocaml.org/t/decoding-many-unicode-strings-with-uutf/8910/2

Also avoid allocations if string is all utf8

This could be further optimized to stop on first invalid utf8 char and
do the recode on the rest.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
@edwintorok edwintorok force-pushed the private/edvint/xsi-xenopsd branch from 2128e9d to 792fdd7 Compare December 6, 2021 14:45
@edwintorok edwintorok marked this pull request as ready for review December 6, 2021 14:45
@edwintorok
Copy link
Contributor Author

Dropped the memory logging and RRD changes, keeping just the 2 leak fixes (the thread, and memory leak), and the utf8 optimization.

@edwintorok edwintorok merged commit ad12c5c into xapi-project:0.17-lcm Dec 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants