{"action":"created","comment":{"url":"https://api.github.com/repos/graalvm/truffle/pulls/comments/52876187","id":52876187,"diff_hunk":"@@ -76,6 +83,30 @@\n         this.startColumn = startColumn;\n         this.charIndex = charIndex;\n         this.charLength = charLength;\n+        this.tags = tags;\n+        assert tagsAreNonNullAndInterned(tags) : \"All tags set for a source section must be interned and non-null.\";\n+    }\n+\n+    @SuppressFBWarnings(\"ES_COMPARING_STRINGS_WITH_EQ\")\n+    private static boolean tagsAreNonNullAndInterned(String[] tags) {\n+        for (int i = 0; i < tags.length; i++) {\n+            if (tags[i] == null) {\n+                return false;\n+            }\n+            if (tags[i].intern() != tags[i]) {\n+                return false;\n+            }\n+        }\n+        return true;\n+    }\n+\n+    /**\n+     * Returns a set of tags that are associated with this source section as array. The returned\n+     * array must be considered read-only. All elements of the array were asserted to be non-null\n+     * and interned for faster comparison.\n+     */\n+    public String[] getTags() {","path":"truffle/com.oracle.truffle.api/src/com/oracle/truffle/api/source/SourceSection.java","position":64,"original_position":64,"commit_id":"4592e4936b3c19a9e578477baab474387b0947d2","original_commit_id":"fa6dead5d917ea93908454f951cf485025639f40","user":{"login":"jtulach","id":1842422,"avatar_url":"https://avatars.githubusercontent.com/u/1842422?v=3","gravatar_id":"","url":"https://api.github.com/users/jtulach","html_url":"https://github.com/jtulach","followers_url":"https://api.github.com/users/jtulach/followers","following_url":"https://api.github.com/users/jtulach/following{/other_user}","gists_url":"https://api.github.com/users/jtulach/gists{/gist_id}","starred_url":"https://api.github.com/users/jtulach/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jtulach/subscriptions","organizations_url":"https://api.github.com/users/jtulach/orgs","repos_url":"https://api.github.com/users/jtulach/repos","events_url":"https://api.github.com/users/jtulach/events{/privacy}","received_events_url":"https://api.github.com/users/jtulach/received_events","type":"User","site_admin":false},"body":"If our goal is to turn **Truffle** from an academic research into a product, we need to stop the *\"try & deprecate later\"* attitude. API design is about balancing freedom - freedom of API designer and freedom of API users. In this case we (as API designers) are opening up twice:\r\n\r\n- by allowing users to use any string as a tag\r\n- by allowing any user to read the tags associated with any SourceSection\r\n\r\nAs I mentioned at initial review, I believe using strings (e.g. case 1) is suboptimal. I am (as a minority) advising not to do it. \r\n\r\nI believe giving everyone a chance to read the tags (e.g. case 2) is even bigger mistake and will paint us into a corner. I am strongly advising not to do it.\r\n\r\nBtw.  I may even share the passion to allow access to all data provided in the constructor, but I don't think we have use-cases for that at the moment. On contrary - we are unsure whether the data passed in should even be there...\r\n\r\nAnyway I am not going to block anything. If your desire is to give up on your freedom in these two areas, it is your decision. I just want you to understand the consequences - e.g. once you give this freedom to API users, I promise to fight on their behalf to make sure their just gained freedom is *never taken away* from them.\r\n","created_at":"2016-02-15T09:33:11Z","updated_at":"2016-02-15T09:33:11Z","html_url":"https://github.com/graalvm/truffle/pull/1#discussion_r52876187","pull_request_url":"https://api.github.com/repos/graalvm/truffle/pulls/1","_links":{"self":{"href":"https://api.github.com/repos/graalvm/truffle/pulls/comments/52876187"},"html":{"href":"https://github.com/graalvm/truffle/pull/1#discussion_r52876187"},"pull_request":{"href":"https://api.github.com/repos/graalvm/truffle/pulls/1"}}},"pull_request":{"url":"https://api.github.com/repos/graalvm/truffle/pulls/1","id":56281148,"html_url":"https://github.com/graalvm/truffle/pull/1","diff_url":"https://github.com/graalvm/truffle/pull/1.diff","patch_url":"https://github.com/graalvm/truffle/pull/1.patch","issue_url":"https://api.github.com/repos/graalvm/truffle/issues/1","number":1,"state":"open","locked":false,"title":"Feature/instrumentation api","user":{"login":"chumer","id":4346215,"avatar_url":"https://avatars.githubusercontent.com/u/4346215?v=3","gravatar_id":"","url":"https://api.github.com/users/chumer","html_url":"https://github.com/chumer","followers_url":"https://api.github.com/users/chumer/followers","following_url":"https://api.github.com/users/chumer/following{/other_user}","gists_url":"https://api.github.com/users/chumer/gists{/gist_id}","starred_url":"https://api.github.com/users/chumer/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/chumer/subscriptions","organizations_url":"https://api.github.com/users/chumer/orgs","repos_url":"https://api.github.com/users/chumer/repos","events_url":"https://api.github.com/users/chumer/events{/privacy}","received_events_url":"https://api.github.com/users/chumer/received_events","type":"User","site_admin":false},"body":"Here it is: the new instrumentation framework. It is the result of several sessions/discussions/walkthroughs with [~mlvdv] and [~jtulach].\r\n\r\n*Reading Recommendation:*\r\n\r\nIf you are not interested in the details skip over to the example uses in com.oracle.truffle.api.instrumentation.examples they should give the best idea.\r\n\r\nFor everybody else:\r\n1) Start reading with the package-info.java and then follow the references to explore the API. \r\n2) Look at the changes in PolyglotEngine on how users can instrument their applications.\r\n3) Look the examples in com.oracle.truffle.api.instrumentation.examples.\r\n4) Look at the changes to SimpleLanguage on how to see how languages can adopt it.\r\n5) Look at the instrumentation tests\r\n6) Look at the instrumentation implementation in InstrumentationHandler\r\n7) Look at changes to DefaultCallTarget to see how lazy instrumentation works\r\n8) Look at the implementation of the annotation processors: GenerateWrapperNodeProcessor and InstrumentationRegistrationProcessor.\r\n\r\nThere is also a trace mode (InstrumentationHandler#TRACE) that should make it easier to understand what is going on.\r\n\r\n\r\n*New and Noteworthy:*\r\no) Filter expression language for source sections with tags that do all the syntax probing/synchronization automatically and language agnostically (see SourceSectionFilter, Instrumenter).\r\no) Everything is now lazy. Wrappers are created only for places which are actually instrumented and which were executed at least once. Wrappers are updated lazy on next/first execution and removed if not needed anymore.\r\no) Fixed set of instrumentation tags for guest languages to adopt.\r\no) Event bindings for language implementations that have special privileges to implement language features with the framework (see InstrumentationLanguage).\r\no) Creation/disposal of instruments in PolyglotEngine and out,err,in for instrumentations.\r\no) Only instrumented RootNode instances are tracked. No tracking of anything else.\r\no) Easy dispose of event bindings. Automatic binding dispose for instrumentations on removal.\r\no) Language agnostic annotation processor for generating wrappers (see @Instrumentable, SL implementation)\r\n\r\n\r\n*Specific review questions:*\r\n\r\nA) Which use cases do you have that are not yet covered in the API?\r\nB) Which use cases do you see that are now covered which were not covered before?\r\nC) How can the implementation/API be improved/simplified?\r\nD) How could the naming be improved? (consistency, intuition, the hardest part :-) )\r\nE) Do you see issues in evolving of this API?\r\nF) How can the documentation be improved?\r\nG) Do you have ideas for tools (try to build them)?\r\n\r\n*Open/Known Issues:*\r\n\r\na) The Registration annotation processor does not work with incremental compiling.\r\n\r\n*Migration strategy/Next steps:*\r\n\r\nAfter taking your feedback into account I plan to push this through (if nobody objects). The old instrumentation framework will then be deprecated and removed later on. Old and new can live side-by-side for a while during adoption of the languages.\r\n\r\nThanks in advance to the reviewers.","created_at":"2016-01-17T16:21:10Z","updated_at":"2016-02-15T09:33:11Z","closed_at":null,"merged_at":null,"merge_commit_sha":"030e0f4f2c204b5120277821055331a2f1ea822b","assignee":{"login":"chumer","id":4346215,"avatar_url":"https://avatars.githubusercontent.com/u/4346215?v=3","gravatar_id":"","url":"https://api.github.com/users/chumer","html_url":"https://github.com/chumer","followers_url":"https://api.github.com/users/chumer/followers","following_url":"https://api.github.com/users/chumer/following{/other_user}","gists_url":"https://api.github.com/users/chumer/gists{/gist_id}","starred_url":"https://api.github.com/users/chumer/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/chumer/subscriptions","organizations_url":"https://api.github.com/users/chumer/orgs","repos_url":"https://api.github.com/users/chumer/repos","events_url":"https://api.github.com/users/chumer/events{/privacy}","received_events_url":"https://api.github.com/users/chumer/received_events","type":"User","site_admin":false},"milestone":null,"commits_url":"https://api.github.com/repos/graalvm/truffle/pulls/1/commits","review_comments_url":"https://api.github.com/repos/graalvm/truffle/pulls/1/comments","review_comment_url":"https://api.github.com/repos/graalvm/truffle/pulls/comments{/number}","comments_url":"https://api.github.com/repos/graalvm/truffle/issues/1/comments","statuses_url":"https://api.github.com/repos/graalvm/truffle/statuses/4592e4936b3c19a9e578477baab474387b0947d2","head":{"label":"graalvm:feature/instrumentation_api","ref":"feature/instrumentation_api","sha":"4592e4936b3c19a9e578477baab474387b0947d2","user":{"login":"graalvm","id":10176827,"avatar_url":"https://avatars.githubusercontent.com/u/10176827?v=3","gravatar_id":"","url":"https://api.github.com/users/graalvm","html_url":"https://github.com/graalvm","followers_url":"https://api.github.com/users/graalvm/followers","following_url":"https://api.github.com/users/graalvm/following{/other_user}","gists_url":"https://api.github.com/users/graalvm/gists{/gist_id}","starred_url":"https://api.github.com/users/graalvm/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/graalvm/subscriptions","organizations_url":"https://api.github.com/users/graalvm/orgs","repos_url":"https://api.github.com/users/graalvm/repos","events_url":"https://api.github.com/users/graalvm/events{/privacy}","received_events_url":"https://api.github.com/users/graalvm/received_events","type":"Organization","site_admin":false},"repo":{"id":49662222,"name":"truffle","full_name":"graalvm/truffle","owner":{"login":"graalvm","id":10176827,"avatar_url":"https://avatars.githubusercontent.com/u/10176827?v=3","gravatar_id":"","url":"https://api.github.com/users/graalvm","html_url":"https://github.com/graalvm","followers_url":"https://api.github.com/users/graalvm/followers","following_url":"https://api.github.com/users/graalvm/following{/other_user}","gists_url":"https://api.github.com/users/graalvm/gists{/gist_id}","starred_url":"https://api.github.com/users/graalvm/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/graalvm/subscriptions","organizations_url":"https://api.github.com/users/graalvm/orgs","repos_url":"https://api.github.com/users/graalvm/repos","events_url":"https://api.github.com/users/graalvm/events{/privacy}","received_events_url":"https://api.github.com/users/graalvm/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/graalvm/truffle","description":"The Truffle Language Implementation Framework","fork":false,"url":"https://api.github.com/repos/graalvm/truffle","forks_url":"https://api.github.com/repos/graalvm/truffle/forks","keys_url":"https://api.github.com/repos/graalvm/truffle/keys{/key_id}","collaborators_url":"https://api.github.com/repos/graalvm/truffle/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/graalvm/truffle/teams","hooks_url":"https://api.github.com/repos/graalvm/truffle/hooks","issue_events_url":"https://api.github.com/repos/graalvm/truffle/issues/events{/number}","events_url":"https://api.github.com/repos/graalvm/truffle/events","assignees_url":"https://api.github.com/repos/graalvm/truffle/assignees{/user}","branches_url":"https://api.github.com/repos/graalvm/truffle/branches{/branch}","tags_url":"https://api.github.com/repos/graalvm/truffle/tags","blobs_url":"https://api.github.com/repos/graalvm/truffle/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/graalvm/truffle/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/graalvm/truffle/git/refs{/sha}","trees_url":"https://api.github.com/repos/graalvm/truffle/git/trees{/sha}","statuses_url":"https://api.github.com/repos/graalvm/truffle/statuses/{sha}","languages_url":"https://api.github.com/repos/graalvm/truffle/languages","stargazers_url":"https://api.github.com/repos/graalvm/truffle/stargazers","contributors_url":"https://api.github.com/repos/graalvm/truffle/contributors","subscribers_url":"https://api.github.com/repos/graalvm/truffle/subscribers","subscription_url":"https://api.github.com/repos/graalvm/truffle/subscription","commits_url":"https://api.github.com/repos/graalvm/truffle/commits{/sha}","git_commits_url":"https://api.github.com/repos/graalvm/truffle/git/commits{/sha}","comments_url":"https://api.github.com/repos/graalvm/truffle/comments{/number}","issue_comment_url":"https://api.github.com/repos/graalvm/truffle/issues/comments{/number}","contents_url":"https://api.github.com/repos/graalvm/truffle/contents/{+path}","compare_url":"https://api.github.com/repos/graalvm/truffle/compare/{base}...{head}","merges_url":"https://api.github.com/repos/graalvm/truffle/merges","archive_url":"https://api.github.com/repos/graalvm/truffle/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/graalvm/truffle/downloads","issues_url":"https://api.github.com/repos/graalvm/truffle/issues{/number}","pulls_url":"https://api.github.com/repos/graalvm/truffle/pulls{/number}","milestones_url":"https://api.github.com/repos/graalvm/truffle/milestones{/number}","notifications_url":"https://api.github.com/repos/graalvm/truffle/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/graalvm/truffle/labels{/name}","releases_url":"https://api.github.com/repos/graalvm/truffle/releases{/id}","deployments_url":"https://api.github.com/repos/graalvm/truffle/deployments","created_at":"2016-01-14T17:11:35Z","updated_at":"2016-02-14T22:32:00Z","pushed_at":"2016-02-14T17:39:31Z","git_url":"git://github.com/graalvm/truffle.git","ssh_url":"git@github.com:graalvm/truffle.git","clone_url":"https://github.com/graalvm/truffle.git","svn_url":"https://github.com/graalvm/truffle","homepage":"","size":20466,"stargazers_count":25,"watchers_count":25,"language":"Java","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":14,"mirror_url":null,"open_issues_count":10,"forks":14,"open_issues":10,"watchers":25,"default_branch":"master"}},"base":{"label":"graalvm:master","ref":"master","sha":"dbba9dedf44dd3096bb21aad65799c9a4d295a43","user":{"login":"graalvm","id":10176827,"avatar_url":"https://avatars.githubusercontent.com/u/10176827?v=3","gravatar_id":"","url":"https://api.github.com/users/graalvm","html_url":"https://github.com/graalvm","followers_url":"https://api.github.com/users/graalvm/followers","following_url":"https://api.github.com/users/graalvm/following{/other_user}","gists_url":"https://api.github.com/users/graalvm/gists{/gist_id}","starred_url":"https://api.github.com/users/graalvm/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/graalvm/subscriptions","organizations_url":"https://api.github.com/users/graalvm/orgs","repos_url":"https://api.github.com/users/graalvm/repos","events_url":"https://api.github.com/users/graalvm/events{/privacy}","received_events_url":"https://api.github.com/users/graalvm/received_events","type":"Organization","site_admin":false},"repo":{"id":49662222,"name":"truffle","full_name":"graalvm/truffle","owner":{"login":"graalvm","id":10176827,"avatar_url":"https://avatars.githubusercontent.com/u/10176827?v=3","gravatar_id":"","url":"https://api.github.com/users/graalvm","html_url":"https://github.com/graalvm","followers_url":"https://api.github.com/users/graalvm/followers","following_url":"https://api.github.com/users/graalvm/following{/other_user}","gists_url":"https://api.github.com/users/graalvm/gists{/gist_id}","starred_url":"https://api.github.com/users/graalvm/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/graalvm/subscriptions","organizations_url":"https://api.github.com/users/graalvm/orgs","repos_url":"https://api.github.com/users/graalvm/repos","events_url":"https://api.github.com/users/graalvm/events{/privacy}","received_events_url":"https://api.github.com/users/graalvm/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/graalvm/truffle","description":"The Truffle Language Implementation Framework","fork":false,"url":"https://api.github.com/repos/graalvm/truffle","forks_url":"https://api.github.com/repos/graalvm/truffle/forks","keys_url":"https://api.github.com/repos/graalvm/truffle/keys{/key_id}","collaborators_url":"https://api.github.com/repos/graalvm/truffle/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/graalvm/truffle/teams","hooks_url":"https://api.github.com/repos/graalvm/truffle/hooks","issue_events_url":"https://api.github.com/repos/graalvm/truffle/issues/events{/number}","events_url":"https://api.github.com/repos/graalvm/truffle/events","assignees_url":"https://api.github.com/repos/graalvm/truffle/assignees{/user}","branches_url":"https://api.github.com/repos/graalvm/truffle/branches{/branch}","tags_url":"https://api.github.com/repos/graalvm/truffle/tags","blobs_url":"https://api.github.com/repos/graalvm/truffle/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/graalvm/truffle/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/graalvm/truffle/git/refs{/sha}","trees_url":"https://api.github.com/repos/graalvm/truffle/git/trees{/sha}","statuses_url":"https://api.github.com/repos/graalvm/truffle/statuses/{sha}","languages_url":"https://api.github.com/repos/graalvm/truffle/languages","stargazers_url":"https://api.github.com/repos/graalvm/truffle/stargazers","contributors_url":"https://api.github.com/repos/graalvm/truffle/contributors","subscribers_url":"https://api.github.com/repos/graalvm/truffle/subscribers","subscription_url":"https://api.github.com/repos/graalvm/truffle/subscription","commits_url":"https://api.github.com/repos/graalvm/truffle/commits{/sha}","git_commits_url":"https://api.github.com/repos/graalvm/truffle/git/commits{/sha}","comments_url":"https://api.github.com/repos/graalvm/truffle/comments{/number}","issue_comment_url":"https://api.github.com/repos/graalvm/truffle/issues/comments{/number}","contents_url":"https://api.github.com/repos/graalvm/truffle/contents/{+path}","compare_url":"https://api.github.com/repos/graalvm/truffle/compare/{base}...{head}","merges_url":"https://api.github.com/repos/graalvm/truffle/merges","archive_url":"https://api.github.com/repos/graalvm/truffle/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/graalvm/truffle/downloads","issues_url":"https://api.github.com/repos/graalvm/truffle/issues{/number}","pulls_url":"https://api.github.com/repos/graalvm/truffle/pulls{/number}","milestones_url":"https://api.github.com/repos/graalvm/truffle/milestones{/number}","notifications_url":"https://api.github.com/repos/graalvm/truffle/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/graalvm/truffle/labels{/name}","releases_url":"https://api.github.com/repos/graalvm/truffle/releases{/id}","deployments_url":"https://api.github.com/repos/graalvm/truffle/deployments","created_at":"2016-01-14T17:11:35Z","updated_at":"2016-02-14T22:32:00Z","pushed_at":"2016-02-14T17:39:31Z","git_url":"git://github.com/graalvm/truffle.git","ssh_url":"git@github.com:graalvm/truffle.git","clone_url":"https://github.com/graalvm/truffle.git","svn_url":"https://github.com/graalvm/truffle","homepage":"","size":20466,"stargazers_count":25,"watchers_count":25,"language":"Java","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":14,"mirror_url":null,"open_issues_count":10,"forks":14,"open_issues":10,"watchers":25,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/graalvm/truffle/pulls/1"},"html":{"href":"https://github.com/graalvm/truffle/pull/1"},"issue":{"href":"https://api.github.com/repos/graalvm/truffle/issues/1"},"comments":{"href":"https://api.github.com/repos/graalvm/truffle/issues/1/comments"},"review_comments":{"href":"https://api.github.com/repos/graalvm/truffle/pulls/1/comments"},"review_comment":{"href":"https://api.github.com/repos/graalvm/truffle/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/graalvm/truffle/pulls/1/commits"},"statuses":{"href":"https://api.github.com/repos/graalvm/truffle/statuses/4592e4936b3c19a9e578477baab474387b0947d2"}}}}