module: improve getPackageType performance#57599
Merged
nodejs-github-bot merged 3 commits intonodejs:mainfrom Apr 6, 2025
Merged
module: improve getPackageType performance#57599nodejs-github-bot merged 3 commits intonodejs:mainfrom
getPackageType performance#57599nodejs-github-bot merged 3 commits intonodejs:mainfrom
Conversation
Collaborator
|
Review requested:
|
nodejs-github-bot
added
c++
dario-piotrowicz
force-pushed
the
dario/add-getPackageType-module-binding
branch
from
759229d to
2c75280
Compare
Member
|
I'll review tomorrow morning. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #57599 +/- ##
==========================================
- Coverage 90.22% 90.22% -0.01%
==========================================
Files 630 630
Lines 185055 185064 +9
Branches 36216 36223 +7
==========================================
- Hits 166975 166969 -6
- Misses 11042 11043 +1
- Partials 7038 7052 +14
🚀 New features to boost your workflow:
|
dario-piotrowicz
force-pushed
the
dario/add-getPackageType-module-binding
branch
3 times, most recently
from
2052d4a to
3406431
Compare
jasnell
reviewed
Mar 24, 2025
jasnell
reviewed
Mar 24, 2025
dario-piotrowicz
force-pushed
the
dario/add-getPackageType-module-binding
branch
from
3406431 to
0670e6e
Compare
dario-piotrowicz
force-pushed
the
dario/add-getPackageType-module-binding
branch
from
0670e6e to
46649e7
Compare
jasnell
reviewed
Mar 25, 2025
Co-authored-by: James M Snell <jasnell@gmail.com>
Member
|
I think this should be ready to go but we're unable to run jenkins CI on it currently due to some ongoing maintenance, Once the CI system is unrestricted again we'll get the CI run scheduled so we can get this landed. |
jasnell
added
the
author ready
anonrig
approved these changes
Mar 28, 2025
This comment was marked as outdated.
This comment was marked as outdated.
UlisesGascon
approved these changes
Mar 31, 2025
Collaborator
Collaborator
Collaborator
aduh95
added
commit-queue
nodejs-github-bot
removed
the
commit-queue
Collaborator
|
Landed in 8c25465 |
JonasBa
pushed a commit
to JonasBa/node
that referenced
this pull request
Apr 11, 2025
PR-URL: nodejs#57599 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com>
RafaelGSS
pushed a commit
that referenced
this pull request
May 1, 2025
PR-URL: #57599 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com>
RafaelGSS
pushed a commit
that referenced
this pull request
May 2, 2025
PR-URL: #57599 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com>
aduh95
pushed a commit
that referenced
this pull request
May 6, 2025
PR-URL: #57599 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com>
RafaelGSS
pushed a commit
that referenced
this pull request
May 14, 2025
PR-URL: #57599 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com>
aduh95
pushed a commit
that referenced
this pull request
May 16, 2025
PR-URL: #57599 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com>
aduh95
pushed a commit
that referenced
this pull request
May 17, 2025
PR-URL: #57599 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com>
aduh95
pushed a commit
that referenced
this pull request
May 17, 2025
PR-URL: #57599 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com>
aduh95
pushed a commit
that referenced
this pull request
May 17, 2025
PR-URL: #57599 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com>
aduh95
pushed a commit
that referenced
this pull request
May 19, 2025
PR-URL: #57599 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com>
ghost
mentioned this pull request
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
packageJsonReader#getPackageTypecurrently gets the whole package scope config from the C++ binding and then extracts itstypefield, my understanding is that this is wasteful (since the whole object needs to be serialized to cross the C++/JS boundary) and that gathering the type in C++ and returning only that would be more efficient, that's what I'm attempting to do in this PR 🙂 (I hope my C++ makes sense as I am pretty inexperienced with it 🤞)