Skip to content

Fix optimized element access that incorrectly reports that the instance has no such property#841

Merged
chakrabot merged 1 commit into
chakra-core:release/1.2from
pleath:isjsnative2
May 4, 2016
Merged

Fix optimized element access that incorrectly reports that the instance has no such property#841
chakrabot merged 1 commit into
chakra-core:release/1.2from
pleath:isjsnative2

Conversation

@pleath
Copy link
Copy Markdown
Contributor

@pleath pleath commented Apr 20, 2016

Reimplement shortcut in element access on unknown property name. The optimization requires a prototype chain walk to confirm that the property being looked for can't be found on an object such as a CEO that may use names unknown to the JS engine, or that, like a proxy, may require that the lookup be done regardless. For now the prototype walk is done on every attempted access where no property record exists for the index name. This could be optimized with a scheme similar to the one we use for non-writable properties in the prototype chain, but I'd like to see a needful use case before I saddle us with another registration mechanism. Benchmarks are flat.

@pleath
Copy link
Copy Markdown
Contributor Author

pleath commented Apr 22, 2016

@Yongqu @ianwjhalliday Reimplemented this so that we only do extra work when we've discovered that no PropertyRecord exists for the given property name. Seems to get rid of perf regressions.

@pleath
Copy link
Copy Markdown
Contributor Author

pleath commented Apr 27, 2016

Ping @Yongqu @ianwjhalliday

{
// If the instance doesn't have the property, typeof result is "undefined".
threadContext->CheckAndResetImplicitCallAccessorFlag();
threadContext->AddImplicitCallFlags(savedImplicitCallFlags);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how do we know there is implicit call?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is conservative; I don't know why it was originally added.

@Yongqu
Copy link
Copy Markdown
Contributor

Yongqu commented Apr 28, 2016

👍

…optimization requires a prototype chain walk to confirm that the property being looked for can't be found on an object such as a CEO that may use names unknown to the JS engine, or that, like a proxy, may require that the lookup be done regardless. For now the prototype walk is done on every attempted access where no property record exists for the index name. This could be optimized with a scheme similar to the one we use for non-writable properties in the prototype chain, but I'd like to see a needful use case before I saddle us with another registration mechanism. Benchmarks are flat.
@chakrabot chakrabot merged commit b2ce371 into chakra-core:release/1.2 May 4, 2016
chakrabot pushed a commit that referenced this pull request May 4, 2016
…at the instance has no such property

Merge pull request #841 from pleath:isjsnative2
Reimplement shortcut in element access on unknown property name. The optimization requires a prototype chain walk to confirm that the property being looked for can't be found on an object such as a CEO that may use names unknown to the JS engine, or that, like a proxy, may require that the lookup be done regardless. For now the prototype walk is done on every attempted access where no property record exists for the index name. This could be optimized with a scheme similar to the one we use for non-writable properties in the prototype chain, but I'd like to see a needful use case before I saddle us with another registration mechanism. Benchmarks are flat.
chakrabot pushed a commit that referenced this pull request May 4, 2016
…ly reports that the instance has no such property

Merge pull request #841 from pleath:isjsnative2
Reimplement shortcut in element access on unknown property name. The optimization requires a prototype chain walk to confirm that the property being looked for can't be found on an object such as a CEO that may use names unknown to the JS engine, or that, like a proxy, may require that the lookup be done regardless. For now the prototype walk is done on every attempted access where no property record exists for the index name. This could be optimized with a scheme similar to the one we use for non-writable properties in the prototype chain, but I'd like to see a needful use case before I saddle us with another registration mechanism. Benchmarks are flat.
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.

4 participants