refactor(NODE-6616): short circuit EJSON stringifying #4360
Conversation
nbbeeken
changed the title
nbbeeken
force-pushed
the
NODE-6616-ejson-max-len
branch
from
4bc5283 to
9e2721f
Compare
nbbeeken
force-pushed
the
NODE-6616-ejson-max-len
branch
from
9e2721f to
0e83e17
Compare
src/mongo_logger.ts
Outdated
| if (typeof value === 'string') { | ||
| currentLength += value.length; | ||
| } else if (typeof value === 'number' || typeof value === 'bigint') { | ||
| currentLength += 20; |
There was a problem hiding this comment.
where is this lengths coming from? wouldn't the length of numbers be dependent on the actual number being stringified (ex: 0 would be 1, but 1.1234 would be 6?)
There was a problem hiding this comment.
Yes correct they would depend on the value. I can improve this to calculate the number of digits from the value.
Generally, the lengths here are approximations when we can't determine the length without actually stringifying the value in an attempt to avoid paying that potentially expensive cost. With a better implementation, we should always get a bit over maxLen returned to us and then slicing ensures we actually hit the target.
nbbeeken
force-pushed
the
NODE-6616-ejson-max-len
branch
from
0e83e17 to
d588ada
Compare
W-A-James
force-pushed
the
NODE-6616-ejson-max-len
branch
from
27d615f to
31f941e
Compare
src/mongo_logger.ts
Outdated
| if (v.scope == null) { | ||
| currentLength += v.code.length + 10 + 2; | ||
| } else { | ||
| // Ignoring actual scope object |
There was a problem hiding this comment.
Do we want to account for the scope?
Description
What is changing?
Is there new documentation needed for these changes?
What is the motivation for this change?
Release Highlight
Fill in title or leave empty for no highlight
Double check the following
npm run check:lintscripttype(NODE-xxxx)[!]: descriptionfeat(NODE-1234)!: rewriting everything in coffeescript