Skip to content

Save 8/4 bytes per node instance. Use is operator instead of bool field #302

Merged
alex-kulakov merged 1 commit intoDataObjects-NET:masterfrom
servicetitan:upstream/optimizeTextNode
Aug 17, 2023
Merged

Save 8/4 bytes per node instance. Use is operator instead of bool field #302
alex-kulakov merged 1 commit intoDataObjects-NET:masterfrom
servicetitan:upstream/optimizeTextNode

Conversation

@SergeiPavlov
Copy link
Copy Markdown
Contributor

Potentially is saves megabytes

image

@SergeiPavlov SergeiPavlov changed the title Save 8/4 bytes per node instance Save 8/4 bytes per node instance. Use is operator instead of bool field Jan 14, 2023
@alex-kulakov
Copy link
Copy Markdown
Contributor

It is 29% slower operation, according to my benchmark runs. You hurt performance for memory. Do you struggle for memory that much?

@SergeiPavlov
Copy link
Copy Markdown
Contributor Author

It is 29% slower operation, according to my benchmark runs.

Does this measuremetn include GC-time?
My experience says that GC is one of main factors of performances.
So I have a bias to save more memory at the cost of CPU cycles

@alex-kulakov
Copy link
Copy Markdown
Contributor

Does this measuremetn include GC-time?

No, it doesn't, I wanted to know what is pure operations cost.

So I have a bias to save more memory at the cost of CPU cycles.

Not always, we know it :)

OK, lets not have the field.

Since a Node is created less frequent than checked for its TextNodeness during conversion to command text :) I wanted to cache this info to improve translation because it happens a lot of times for cached queries, it reduces translation overhead for a bit for the cost of size of this field. This was my thoughts, to me faster queries are important, little bit here, little bit there and we have some significant value :)

If this is ok for you highly loaded app then I agree.

alex-kulakov
alex-kulakov previously approved these changes Aug 17, 2023
@alex-kulakov alex-kulakov changed the base branch from master to 7.1 August 17, 2023 08:15
@alex-kulakov alex-kulakov dismissed their stale review August 17, 2023 08:15

The base branch was changed.

@alex-kulakov alex-kulakov changed the base branch from 7.1 to master August 17, 2023 08:16
@alex-kulakov alex-kulakov merged commit 2862041 into DataObjects-NET:master Aug 17, 2023
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.

2 participants