Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

os: performance improvement in vector allocation #36748

Closed
wants to merge 1 commit into from

Conversation

yashLadha
Copy link
Contributor

We were using the result vector with an object which is not a primitive
data type, and going with the constructor allocation pattern it creates
a size of that vector and also initializes the spaces with the data type
as well which is in our case is Local<Value>. It leads to waste of
some CPU cycles and instead we just wanted to have some reserved space
in our vector.

We can use reserve method on vector to reserve some space for the
vector but doesn't initialize the value since we are anyways doing it in
the following loop.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. os Issues and PRs related to the os subsystem. labels Jan 3, 2021
@yashLadha yashLadha force-pushed the improve_vector_allocation branch from d3d391f to 6dc0a6e Compare January 3, 2021 07:18
We were using the result vector with an object which is not a primitive
data type, and going with the constructor allocation pattern it creates
a size of that vector and also initializes the spaces with the data type
as well which is in our case is `Local<Value>`. It leads to waste of
some CPU cycles and instead we just wanted to have some reserved space
in our vector.

We can use `reserve` method on vector to reserve some space for the
vector but doesn't initialize the value since we are anyways doing it in
the following loop.
@yashLadha yashLadha force-pushed the improve_vector_allocation branch from 6dc0a6e to aa24929 Compare January 3, 2021 07:23
@yashLadha
Copy link
Contributor Author

@yashLadha yashLadha added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 5, 2021
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 5, 2021
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@aduh95 aduh95 added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 18, 2021
@jasnell
Copy link
Member

jasnell commented Jan 18, 2021

Landed in cb2535e

@jasnell jasnell closed this Jan 18, 2021
jasnell pushed a commit that referenced this pull request Jan 18, 2021
We were using the result vector with an object which is not a primitive
data type, and going with the constructor allocation pattern it creates
a size of that vector and also initializes the spaces with the data type
as well which is in our case is `Local<Value>`. It leads to waste of
some CPU cycles and instead we just wanted to have some reserved space
in our vector.

We can use `reserve` method on vector to reserve some space for the
vector but doesn't initialize the value since we are anyways doing it in
the following loop.

PR-URL: #36748
Reviewed-By: Robert Nagy <[email protected]>
Reviewed-By: James M Snell <[email protected]>
ruyadorno pushed a commit that referenced this pull request Jan 22, 2021
We were using the result vector with an object which is not a primitive
data type, and going with the constructor allocation pattern it creates
a size of that vector and also initializes the spaces with the data type
as well which is in our case is `Local<Value>`. It leads to waste of
some CPU cycles and instead we just wanted to have some reserved space
in our vector.

We can use `reserve` method on vector to reserve some space for the
vector but doesn't initialize the value since we are anyways doing it in
the following loop.

PR-URL: #36748
Reviewed-By: Robert Nagy <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@ruyadorno ruyadorno mentioned this pull request Jan 22, 2021
ruyadorno pushed a commit that referenced this pull request Jan 25, 2021
We were using the result vector with an object which is not a primitive
data type, and going with the constructor allocation pattern it creates
a size of that vector and also initializes the spaces with the data type
as well which is in our case is `Local<Value>`. It leads to waste of
some CPU cycles and instead we just wanted to have some reserved space
in our vector.

We can use `reserve` method on vector to reserve some space for the
vector but doesn't initialize the value since we are anyways doing it in
the following loop.

PR-URL: #36748
Reviewed-By: Robert Nagy <[email protected]>
Reviewed-By: James M Snell <[email protected]>
targos pushed a commit that referenced this pull request May 1, 2021
We were using the result vector with an object which is not a primitive
data type, and going with the constructor allocation pattern it creates
a size of that vector and also initializes the spaces with the data type
as well which is in our case is `Local<Value>`. It leads to waste of
some CPU cycles and instead we just wanted to have some reserved space
in our vector.

We can use `reserve` method on vector to reserve some space for the
vector but doesn't initialize the value since we are anyways doing it in
the following loop.

PR-URL: #36748
Reviewed-By: Robert Nagy <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@danielleadams danielleadams mentioned this pull request May 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. os Issues and PRs related to the os subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants