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.

Sign up for GitHub

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

Jump to bottom

buffer: backport --zero-fill-buffers command line option #5745

Closed
wants to merge 1 commit into from
Closed

buffer: backport --zero-fill-buffers command line option #5745

wants to merge 1 commit into from

Conversation

jasnell
Copy link
Member

@jasnell jasnell commented Mar 16, 2016

Pull Request check-list

  • Does make -j8 test (UNIX) or vcbuild test nosign (Windows) pass with
    this change (including linting)?
  • Is the commit message formatted according to [CONTRIBUTING.md][0]?
  • If this change fixes a bug (or a performance problem), is a regression
    test (or a benchmark) included?
  • Is a documentation update included (if this change modifies
    existing APIs, or introduces new ones)?

Affected core subsystem(s)

buffer

Description of change

This backports the --zero-fill-buffers command line flag introduced in master. When used, all Buffer and SlowBuffer instances will zero fill by default.

This does not backport any of the other Buffer API or behavior changes.

Note: My intent is to open similar backports for --zero-fill-buffer in v5, v0.12 and v0.10

/cc @trevnorris

@jasnell jasnell added buffer Issues and PRs related to the buffer subsystem. semver-minor PRs that contain new features and should be released in the next minor version. lts-watch-v4.x labels Mar 16, 2016
@rvagg
Copy link
Member

rvagg commented Mar 16, 2016

I'd like @nodejs/lts to have a discussion about this on 0.12 and 0.10 and whether we can justify it since 0.10 is in maintenance and 0.12 is about to be maintenance.

@jasnell
Copy link
Member Author

jasnell commented Mar 16, 2016

Yep. Agreed. Backporting this to v0.12 and v0.10 is a bit ugly because of
smalloc. Not impossible, just ugly.
On Mar 16, 2016 2:59 PM, "Rod Vagg" notifications@github.com wrote:

I'd like @nodejs/lts https://github.com/orgs/nodejs/teams/lts to have a
discussion about this on 0.12 and 0.10 and whether we can justify it since
0.10 is in maintenance and 0.12 is about to be maintenance.

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#5745 (comment)

@jasnell
Copy link
Member Author

jasnell commented Mar 17, 2016

@nodejs/ctc

@jasnell jasnell force-pushed the v4.x-zero-fill-buffers branch from 111dce9 to 93843f6 Compare March 21, 2016 22:14
@jasnell
Copy link
Member Author

jasnell commented Mar 21, 2016

Realized I flubbed the original push... updated.

@jasnell
Copy link
Member Author

jasnell commented Mar 21, 2016

@nodejs/ctc ... the LTS WG discussed this PR (along with #5748 and #5751) today and decided that the command line flag should land in v4, v0.12 and v0.10. It was decided that the new constructor APIs should be ported to v4 and not to v0.12 or v0.10. Before I get these command line flag PRs landed, we wanted to get a quick sign off from the CTC.

@jasnell
Copy link
Member Author

jasnell commented Mar 23, 2016

CTC decision today to hold off landing this in v4 for now. Will keep the PR open. @nodejs/lts

@MylesBorins
Copy link
Contributor

@jasnell you may want to include the changes that fixed the regressions we found on v5.x

@jasnell
Copy link
Member Author

jasnell commented Apr 11, 2016

@thealphanerd ... yep, I'll be revisiting this later on. For now this is a don't land

@MylesBorins MylesBorins mentioned this pull request May 16, 2016
Closed
@MylesBorins MylesBorins force-pushed the v4.x-staging branch 2 times, most recently from 6b010a9 to 17a41c6 Compare June 24, 2016 00:29
@jasnell
Copy link
Member Author

jasnell commented Jun 27, 2016

@nodejs/ctc ... is it time to go ahead and get this landed? I can get this updated and there's a minor fix to make but can get it landed in time for v4.5

@ChALkeR
Copy link
Member

ChALkeR commented Jun 28, 2016 •
edited
Loading

@jasnell I have a backport of the new Buffer API at https://github.com/ChALkeR/io.js/commits/buffer-v4.x-squashed (unfinished, hope to file a PR soon), and I hope to see a minor release of 4.x with both the new API and the flag backported.

Update: the branch link was wrong, fixed

@rvagg rvagg mentioned this pull request Jun 29, 2016
Closed
@ChALkeR ChALkeR mentioned this pull request Jun 29, 2016
Closed
4 tasks
@jasnell
Copy link
Member Author

jasnell commented Jun 29, 2016

I will be updating this PR this week with an intent towards getting it landed along with #7475

@jasnell
Copy link
Member Author

jasnell commented Jun 29, 2016

@thealphanerd ... I've pulled the don't land label but added an in-progress label. I'll get this rebased and updated tomorrow.

@jasnell
buffer: backport --zero-fill-buffers command line option
f615770 
This backports the --zero-fill-buffers command line flag introduced
in master. When used, all Buffer and SlowBuffer instances will zero
fill by default.

This does *not* backport any of the other Buffer API or behavior
changes.
@jasnell jasnell force-pushed the v4.x-zero-fill-buffers branch from 93843f6 to f615770 Compare July 1, 2016 17:34
@jasnell
Copy link
Member Author

jasnell commented Jul 1, 2016

Updated! New CI: https://ci.nodejs.org/job/node-test-pull-request/3152/

@jasnell
Copy link
Member Author

jasnell commented Jul 1, 2016

CI is green

@jasnell
Copy link
Member Author

jasnell commented Jul 1, 2016

@ChALkeR @thealphanerd @nodejs/ctc ... PTAL

@jasnell jasnell removed the wip Issues and PRs that are still a work in progress. label Jul 1, 2016
@MylesBorins
Copy link
Contributor

@jasnell can you run citgm as well?

@jasnell
Copy link
Member Author

jasnell commented Jul 1, 2016

Yep! https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/334/

@jasnell
Copy link
Member Author

jasnell commented Jul 1, 2016

citgm is green except for one known flaky failure

@jasnell
Copy link
Member Author

jasnell commented Jul 3, 2016

ping @nodejs/ctc ... PTAL

@trevnorris
Copy link
Contributor

LGTM

MylesBorins pushed a commit that referenced this pull request Jul 5, 2016
@jasnell
buffer: backport --zero-fill-buffers cli option
bee7fe5 
This backports the --zero-fill-buffers command line flag introduced
in master. When used, all Buffer and SlowBuffer instances will zero
fill by default.

This does *not* backport any of the other Buffer API or behavior
changes.

PR-URL: #5745
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
@MylesBorins
Copy link
Contributor

landed in bee7fe5

@MylesBorins MylesBorins closed this Jul 5, 2016
@ChALkeR ChALkeR mentioned this pull request Jul 6, 2016
Closed
4 tasks
MylesBorins pushed a commit that referenced this pull request Jul 11, 2016
@jasnell
buffer: backport --zero-fill-buffers cli option
a27eeab 
This backports the --zero-fill-buffers command line flag introduced
in master. When used, all Buffer and SlowBuffer instances will zero
fill by default.

This does *not* backport any of the other Buffer API or behavior
changes.

PR-URL: #5745
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Jul 12, 2016
Closed
MylesBorins pushed a commit that referenced this pull request Jul 12, 2016
@jasnell
buffer: backport --zero-fill-buffers cli option
fb03e57 
This backports the --zero-fill-buffers command line flag introduced
in master. When used, all Buffer and SlowBuffer instances will zero
fill by default.

This does *not* backport any of the other Buffer API or behavior
changes.

PR-URL: #5745
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jul 12, 2016
2016-07-12, Version 4.5.0 'Argon' (LTS)
3103a79 
Notable Changes:

This list is not yet complete. Please comment in the thread with
commits you think should be included. Descriptions will also be
updated in a later release candidate.

Semver Minor:

* buffer:
 * backport new buffer constructor APIs to v4.x
   (Сковорода Никита Андреевич)
   #7562
 * ignore negative allocation lengths (Anna Henningsen)
   #7562
 * backport --zero-fill-buffers cli option (James M Snell)
   #5745
* build:
  * add Intel Vtune profiling support (Chunyang Dai)
    #5527
* repl:
  * copying tabs shouldn't trigger completion (Eugene Obrezkov)
    #5958
* src:
  * add node::FreeEnvironment public API (Cheng Zhao)
    #3098
* test:
  * run v8 tests from node tree (Bryon Leung)
    #4704
* V8:
  * backport 9c927d0f01 from V8 upstream (Myles Borins)
    #7451
  * cherry-pick 68e89fb from v8's upstream (Fedor Indutny)
    #3779

Semver Patch:

* **libuv**:
  * upgrade libuv to 1.9.1 (Saúl Ibarra Corretgé)
    #6796
  * upgrade libuv to 1.9.0 (Saúl Ibarra Corretgé)
    #5994
@MylesBorins MylesBorins mentioned this pull request Jul 12, 2016
Merged
MylesBorins pushed a commit that referenced this pull request Jul 13, 2016
2016-07-12, Version 4.5.0 'Argon' (LTS)
79dd0df 
Notable Changes:

This list is not yet complete. Please comment in the thread with
commits you think should be included. Descriptions will also be
updated in a later release candidate.

Semver Minor:

* buffer:
 * backport new buffer constructor APIs to v4.x
   (Сковорода Никита Андреевич)
   #7562
 * ignore negative allocation lengths (Anna Henningsen)
   #7562
 * backport --zero-fill-buffers cli option (James M Snell)
   #5745
* build:
  * add Intel Vtune profiling support (Chunyang Dai)
    #5527
* repl:
  * copying tabs shouldn't trigger completion (Eugene Obrezkov)
    #5958
* src:
  * add node::FreeEnvironment public API (Cheng Zhao)
    #3098
* test:
  * run v8 tests from node tree (Bryon Leung)
    #4704
* V8:
  * Add post portem data to imrpove object inspection and function's
    context variables inspection (Fedor Indutny)
    #3779

Semver Patch:

* **libuv**:
  * upgrade libuv to 1.9.1 (Saúl Ibarra Corretgé)
    #6796
  * upgrade libuv to 1.9.0 (Saúl Ibarra Corretgé)
    #5994
MylesBorins pushed a commit that referenced this pull request Jul 14, 2016
2016-07-14, Version 4.5.0 'Argon' (LTS)
1dbc727 
Notable Changes:

This list is not yet complete. Please comment in the thread with
commits you think should be included. Descriptions will also be
updated in a later release candidate.

Semver Minor:

* buffer:
 * backport new buffer constructor APIs to v4.x
   (Сковорода Никита Андреевич)
   #7562
 * backport --zero-fill-buffers cli option (James M Snell)
   #5745
* build:
  * add Intel Vtune profiling support (Chunyang Dai)
    #5527
* repl:
  * copying tabs shouldn't trigger completion (Eugene Obrezkov)
    #5958
* src:
  * add node::FreeEnvironment public API (Cheng Zhao)
    #3098
* test:
  * run v8 tests from node tree (Bryon Leung)
    #4704
* V8:
  * Add post portem data to imrpove object inspection and function's
    context variables inspection (Fedor Indutny)
    #3779

Semver Patch:

* **buffer**:
  * ignore negative allocation lengths (Anna Henningsen)
    #7562
* **libuv**:
  * upgrade libuv to 1.9.1 (Saúl Ibarra Corretgé)
    #6796
  * upgrade libuv to 1.9.0 (Saúl Ibarra Corretgé)
    #5994
* **npm**:
  * upgrade to 2.15.9 (Kat Marchán)
    #7692
MylesBorins pushed a commit that referenced this pull request Aug 15, 2016
2016-07-14, Version 4.5.0 'Argon' (LTS)
20ebcf5 
Notable Changes:

Semver Minor:

* buffer:
 * backport new buffer constructor APIs to v4.x
   (Сковорода Никита Андреевич)
   #7562
 * backport --zero-fill-buffers cli option (James M Snell)
   #5745
* build:
  * add Intel Vtune profiling support (Chunyang Dai)
    #5527
* repl:
  * copying tabs shouldn't trigger completion (Eugene Obrezkov)
    #5958
* src:
  * add node::FreeEnvironment public API (Cheng Zhao)
    #3098
* test:
  * run v8 tests from node tree (Bryon Leung)
    #4704
* V8:
  * Add post mortem data to improve object inspection and function's
    context variables inspection (Fedor Indutny)
    #3779

Semver Patch:

* **buffer**:
  * ignore negative allocation lengths (Anna Henningsen)
    #7562
* **crypto**:
  * update root certificates (Ben Noordhuis)
    #7363
* **libuv**:
  * upgrade libuv to 1.9.1 (Saúl Ibarra Corretgé)
    #6796
  * upgrade libuv to 1.9.0 (Saúl Ibarra Corretgé)
    #5994
* **npm**:
  * upgrade to 2.15.9 (Kat Marchán)
    #7692
MylesBorins pushed a commit that referenced this pull request Aug 16, 2016
2016-08-15, Version 4.5.0 'Argon' (LTS)
50577fa 
Notable Changes:

Semver Minor:

* buffer:
 * backport new buffer constructor APIs to v4.x
   (Сковорода Никита Андреевич)
   #7562
 * backport --zero-fill-buffers cli option (James M Snell)
   #5745
* build:
  * add Intel Vtune profiling support (Chunyang Dai)
    #5527
* repl:
  * copying tabs shouldn't trigger completion (Eugene Obrezkov)
    #5958
* src:
  * add node::FreeEnvironment public API (Cheng Zhao)
    #3098
* test:
  * run v8 tests from node tree (Bryon Leung)
    #4704
* V8:
  * Add post mortem data to improve object inspection and function's
    context variables inspection (Fedor Indutny)
    #3779

Semver Patch:

* **buffer**:
  * ignore negative allocation lengths (Anna Henningsen)
    #7562
* **crypto**:
  * update root certificates (Ben Noordhuis)
    #7363
* **libuv**:
  * upgrade libuv to 1.9.1 (Saúl Ibarra Corretgé)
    #6796
  * upgrade libuv to 1.9.0 (Saúl Ibarra Corretgé)
    #5994
* **npm**:
  * upgrade to 2.15.9 (Kat Marchán)
    #7692
MylesBorins pushed a commit that referenced this pull request Aug 16, 2016
2016-08-15, Version 4.5.0 'Argon' (LTS)
e7bd4d2 
Notable Changes:

Semver Minor:

* buffer:
 * backport new buffer constructor APIs to v4.x
   (Сковорода Никита Андреевич)
   #7562
 * backport --zero-fill-buffers cli option (James M Snell)
   #5745
* build:
  * add Intel Vtune profiling support (Chunyang Dai)
    #5527
* repl:
  * copying tabs shouldn't trigger completion (Eugene Obrezkov)
    #5958
* src:
  * add node::FreeEnvironment public API (Cheng Zhao)
    #3098
* test:
  * run v8 tests from node tree (Bryon Leung)
    #4704
* V8:
  * Add post mortem data to improve object inspection and function's
    context variables inspection (Fedor Indutny)
    #3779

Semver Patch:

* **buffer**:
  * ignore negative allocation lengths (Anna Henningsen)
    #7562
* **crypto**:
  * update root certificates (Ben Noordhuis)
    #7363
* **libuv**:
  * upgrade libuv to 1.9.1 (Saúl Ibarra Corretgé)
    #6796
  * upgrade libuv to 1.9.0 (Saúl Ibarra Corretgé)
    #5994
* **npm**:
  * upgrade to 2.15.9 (Kat Marchán)
    #7692
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
buffer Issues and PRs related to the buffer subsystem. semver-minor PRs that contain new features and should be released in the next minor version.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@jasnell @rvagg @MylesBorins @ChALkeR @trevnorris